Skip to content
This repository has been archived by the owner on May 31, 2023. It is now read-only.

bug in array_add #18

Closed
chochkov opened this issue Feb 20, 2014 · 14 comments
Closed

bug in array_add #18

chochkov opened this issue Feb 20, 2014 · 14 comments

Comments

@chochkov
Copy link

@roa

see this https://github.com/adjust/pg-numhstore/compare/array_add_fix?expand=1 for an illustration of a bug that results in us having cohorts with 0 valued keys for apps that make no revenue.

Could you please fix that

@chochkov
Copy link
Author

that thing fails to build

@roa
Copy link
Member

roa commented Feb 20, 2014

what is the expected result of

SELECT array_add(ARRAY[1,2,3]::text[], ARRAY[1,NULL,3]);

?

@chochkov
Copy link
Author

the expected result would be:

  "1"=>"1", "3"=>"3"

@chochkov
Copy link
Author

strangely, there's already a spec that seems to cover NULL args and that one passes, but the one I added doesnt

@roa
Copy link
Member

roa commented Feb 20, 2014

is it possible, that a value can be "0"?

@chochkov
Copy link
Author

From my perspective, 0 should be respected, but NULLs ignored. However, I'd like @rapimo's view on this

@roa
Copy link
Member

roa commented Feb 20, 2014

that is super hard and expensive as we would have to evaluate the array as a c and postgres datatype array. what is the difference between "0" and "NULL"?

@roa
Copy link
Member

roa commented Feb 20, 2014

it is fixed here, if "0" and "NULL" is the same:

#19

@chochkov
Copy link
Author

I think @rapimo should have the final say here.

@rapimo
Copy link
Member

rapimo commented Feb 20, 2014

mmmh yeah not sure for me 0 is a valid value for a numhstore where NULL is a non existent key
so if it doesn't take too much effort I'd love to keep 0

rapimo added a commit that referenced this issue Feb 20, 2014
@rapimo
Copy link
Member

rapimo commented Feb 20, 2014

so @roa can you go ahead ? I added a test case in 8fe486f

@roa
Copy link
Member

roa commented Feb 20, 2014

i just tried:

in an postgres integer array "0" and "NULL" is the same. if you explicitly set it to "0" it is set to NULL in the low level array. so i see no possibility to make that happen as i can not differ between both. any ideas?

@roa
Copy link
Member

roa commented Feb 20, 2014

oh wait, i found something! hold on.

@roa
Copy link
Member

roa commented Feb 20, 2014

@rapimo fixed. can you test if it works on mac os as well?

roa added a commit that referenced this issue Feb 21, 2014
@roa roa closed this as completed in 0306a99 Feb 21, 2014
rapimo added a commit that referenced this issue Feb 21, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants