Adding support for LAST function #3
Conversation
csilvers
left a comment
csilvers
left a comment
There was a problem hiding this comment.
Thanks for doing this! This isn't a big function but it has a surprising number of special cases and it would be great to make sure they're all tested.
| values = [None] | ||
|
|
||
| if column.mode == tq_modes.REPEATED: | ||
| values = [repeated_row[-1] if len(repeated_row) > 0 else None |
There was a problem hiding this comment.
Is it possible for column.values to be empty in repeated mode? If so, I think this comprehension will die because repeated_row will be None which you can't take the len of.
Assuming that situation is possible, can you add a test-case for it?
| values = [None] | ||
|
|
||
| if column.mode == tq_modes.REPEATED: | ||
| values = [repeated_row[-1] if len(repeated_row) > 0 else None |
There was a problem hiding this comment.
Also, looking at this code it looks like the returned value can have NULL's in it if the row is empty. Does this match what bigquery does? The documentation doesn't seem to say.
It would be great if you could have a test-case for that as well.
|
I'll be honest...I just copied and pasted the FIRST function and tests assuming that LAST behaved in the same way...and we're not using LAST any more so this one is lower priority to me. Will leave it for now and come back to it if we need it. |
2 participants
No description provided.