[INSTRM-125] Archiver dies when table does not have any row Created: 15/Jun/17  Updated: 21/Jun/17  Resolved: 21/Jun/17

Status: Done
Project: Instrument control development
Component/s: ics_archiver
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: shimono Assignee: cloomis
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Reviewers: arnaud.lefur

 Description   

pfs@ics-arch:/simdata/ics-arch-201701/products/Linux64/ics_archiver/master-ga619cf6fd9$ ./bin/archiveServer.py
Running ./bin/archiveServer.py as PID 22621
Starting archive server with output to /simdata/ics-arch-201701//logs/actors/archiver/archiver-22621
importing postgres dbapi module psycopg2
reply_raw
Traceback (most recent call last):
File "./bin/archiveServer.py", line 196, in <module>
main()
File "./bin/archiveServer.py", line 193, in main
startServer(options)
File "./bin/archiveServer.py", line 69, in startServer
archiver.database.init(options)
File "/simdata/ics-arch-201701/products/Linux64/ics_archiver/master-ga619cf6fd9/python/archiver/database.py", line 105, in init
print "database: table %s contains %d rows" % (tableName,tableRows)
TypeError: %d format: a number is required, not NoneType

max() will return null value but not 0 for no row inserted. need to add handler when no row exists.



 Comments   
Comment by arnaud.lefur [ 16/Jun/17 ]

Sorry for that, I've been a bit careless.
I've looked at your commit on github:

tableRows = cursor.fetchone()[0]
+        if tableRows is None:
+            tableRows = 0

If you do that you'll still have a

 TypeError: 'NoneType' object has no attribute '__getitem__' 

because it's cursor.fetchone() which return None, I suggest this if you don't mind.

tableRows = cursor.fetchone()
tableRows = tableRows[0] if tableRows is not None else 0
Comment by shimono [ 19/Jun/17 ]

This sounds somehow strange to me. Since SQL is max, it shall return at least one row, and fetchone() shall return that value. If no, there could be another error such as connection or table definition.

Comment by cloomis [ 19/Jun/17 ]

MAX() will return NULL on empty tables. I think a standard fix is to use COALESCE(MAX(...), 0), where COALESCE returns the first non-NULL value in its arguments.

Comment by shimono [ 19/Jun/17 ]

If we want to go SQL, I want to point we would better to use NULLIF...

Comment by arnaud.lefur [ 19/Jun/17 ]

Yes, in fact I was wrong.
COALESCE seems fine, I've just tested it.

cursor.execute("select COALESCE(MAX(%s),0) from %s" % (idLab, tableName))
tableRows = cursor.fetchone()
tableRows = tableRows[0] if tableRows is not None else 0

Should we merge and close that issue ?

Comment by shimono [ 19/Jun/17 ]

I posted a wrong comment sorry. NULLIF is the opposite operation.

For fix, if we put null validation code in SQL, I'd rather propose to arise exception (but not by print) than handling None for fetchone(), since the error shall be invoked or injected well before the SQL just executed. So, I'd propose

cursor.execute("select COALESCE(MAX(%s),0) from %s" % (idLab, tableName))
tableRows = cursor.fetchone()
if tableRows is None:
    raise ValueError('Could not get MAX(%s) from %s' % (idLab, tableName))
else:
    tableRows = tableRows[0]

or similar.

Comment by shimono [ 21/Jun/17 ]

merged.

Generated at Sat Feb 10 16:21:48 JST 2024 using Jira 8.3.4#803005-sha1:1f96e09b3c60279a408a2ae47be3c745f571388b.