PHOENIX-7198 support for multi row constructors in single upsert query
Probably I need to add more tests for this functionality. Please review it, I would like to know what do you think about it.
This could be a nice feature! I am curious about the use case: Is this required for auto-commit connections from sqlline?
@richardantal Is there performance gain from using multiple values in a single query ? I don't think so since we can achieve the same by batching multiple upsert statements and then doing a commit.
@richardantal Is there performance gain from using multiple values in a single query ? I don't think so since we can achieve the same by batching multiple upsert statements and then doing a commit.
My take:
- Avoids network RTT for Query Server
- Avoids multiple query parse and compilation phases
also:
- Adds an previously unimplemented SQL feature.
- Improves compatibility with existing SQL tools that expect thisfeature (though in reality, most of those tools don't handle upserts anyway)
Thanks for the reviews, I added a bit more tests
I also uploaded a simple performance test for this feature to the ticket as an attachment. For me, inserting simple rows took 65 sec, inserting 2 at once 59, 4: 57, 8: 56 (With the same number of commits) So there is a minor performance improvement using this feature but it is not the major point of this feature.
10% improvement for bulk loading is nothing to sneeze at @richardantal .
I rebased the change to resolve conflict with spotless change. Lets wait for a test run
The last CI run still shows spotless issues. Is there anything blocking this @richardantal ?
I rebase the change It shouldn't contain spotless issues anymore
Given the grammar changes, we want this only for 5.4.0 right?
Thanks Viraj for the question, it is a good one. In the PhoenixSQL.g I added an init phase for the valueNodesList. I think it can not be null. https://github.com/richardantal/phoenix-1/blob/PHOENIX-7198/phoenix-core-client/src/main/antlr3/PhoenixSQL.g#L868
I tweaked the above condition a little, to be closer to what we had earlier.