search

citusdata / pg_shard

ATTENTION: pg_shard is superseded by Citus, its more powerful replacement
https://github.com/citusdata/citus
GNU Lesser General Public License v3.0
1.06k stars 63 forks source link

Fix repair bug #120

Closed jasonmp85 closed 9 years ago

jasonmp85 commented 9 years ago

list_union was inappropriate here to begin with; furthermore, it needs the elements of each list to be Nodes. If they are not, we'll get an unsafe cast and (hopefully) an ERROR.

Do not hit "merge" on this… the pull request is against master and so merging would create a new release. I need to bump the version number and squash the test/fix commits before merging.

jasonmp85 commented 9 years ago

Travis is going to build the tip of this branch, so you won't see the actual bug exercised. I built just the failing test locally and here is the bad output:

SELECT master_copy_shard_placement(20, 'localhost', 5432, '127.0.0.1', 5432);
ERROR:  unrecognized node type: 1095062083

After the fix, this line goes away and the test works as before. I added the indexes to the test to force the DDL replay list to contain more than one command (triggering the list_union bug in the description).

onderkalaci commented 9 years ago

@jasonmp85 This change makes sense. As you already mentioned, the correct action is to concat the lists. Again, as you mentioned, list_union is not convenient since it callsequal() which expects the input elements to be Node type, where in our case elements are char * .

I tested the change locally. So, we can :shipit: