| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Greetings! First of all, thank you for your good work and the contribution. So what could be the way forward:
So just maybe, we should keep the Standard Merge and then derive an Enhanced Merge statement from it, probably even as a separate Class and Production. @wumpz @tomershay @AnEmortalKid @jxnu-liguobin @zaza @gitmotte @tvar Your have contributed the most to the project, so please let us know your opinion on this. |
Sorry, something went wrong.
|
Technically this PR is exceptionally well crafted! No concerns from this side. It's really all about the Snowflake specific syntax and how far we want to go under a RDBMS agnostic approach. |
Sorry, something went wrong.
|
Thanks so much for the fast feedback @manticore-projects - all very fair, and I'm happy to make whatever changes we feel are best overall 👍 |
Sorry, something went wrong.
|
I want to add one thought: I am using MERGE intensively on Oracle and H2, Postgres only added it lately. Washing up the syntax may allow for statements appearing valid when not valid on the big four RDBMS. Lets wait a week or so for opinions to hit and then we will make a joint decision. Thank you for understanding. |
Sorry, something went wrong.
|
I have created a Github Poll: #1939 |
Sorry, something went wrong.
I don't see it quite so strictly. A validator (FeatureSetValidation/DatabaseType and FeatureSetValidation/Version) can be built/extended to validate which syntax is correct for which RDMS.
I do not use Oracle, I have defined several validators for several widely used RDBMS. Besides the DatabaseType Validation classes ORACLE, MYSQL, SQLSERVER, MARIADB, POSTGRESQL, H2, there is a SQLVersion class that is supposed to define the ANSI SQL standard syntax. However, not all databases are fully ANSI-compatible for all statements.
Deprecation should only take place if the methods are completely outdated or incorrect. I agree that we should keep them.
The class-structure will never fulfill all the requirements of all RDBMSs, so successful parsing will never guarantee a valid statement for an RDBMS. We have this problem in many cases in this library. |
Sorry, something went wrong.
|
My 2 cents:
I didn't get a chance to look at the specific technical details of this change, so I'm mainly sharing thoughts based on the discussion above and initial understanding of the suggested change. |
Sorry, something went wrong.
|
I think adding one is acceptable if we have standalone support for that database (oracle, mysql,h2,pg). When there is no standalone support (a new database category), it may be more in line with the standard to not accept big changes. I have a similar problem, we are heavily using Doris which uses MySQL driver but still has some special operations and since Doris is not a legacy database and not currently popular, there is no such database category in jsqlparser so I didn't create PR for Doris. |
Sorry, something went wrong.
|
Thank you all for your feedback. I read it s supportive as long as the support for the Standard Merge statement stays intact. @davidjgoss Can you please amend your PR in order to make it as compatible as possible to the Standard Merge. E. g. don't deprecate the existing methods and assume that standard count of merge operations = 1 ( 1or zero INSERT and 1 or zero DELETE) -- unless more operations are either APPENDED or SET with an index. Then I would promptly merge the PR. |
Sorry, something went wrong.
|
@manticore-projects I've pushed a commit that I think addresses this. The original three standard fields in Merge are reinstated including accessors. The new single list remains too, and the setters will cross-update, meaning even with just the strictly standard behaviour we can still use the single list for deparsing and toString(), and consumers can make use of the new visitor methods too. Let me know what you think! |
Sorry, something went wrong.
|
Great! Thank you for your contribution and support, much appreciated! |
Sorry, something went wrong.
|
Congrats, your new Snowflake syntax are online: https://manticore-projects.com/JSQLParser/syntax_snapshot.html#merge |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This PR changes the parser to account for any number of WHEN MATCHED and/or WHEN NOT MATCHED clauses in a MERGE statement. This is most directly applicable to Snowflake. From their docs:
To achieve this, the parser now collects a list of MergeOperations which can be a WHEN MATCHED THEN DELETE (newly added also), WHEN MATCHED THEN UPDATE or WHEN NOT MATCHED THEN INSERT.
The pre-existing getters that presume 0 or 1 of MergeUpdate and MergeInsert are preserved, but deprecated and suggest using the newly added MergeOperationVisitor instead, which is itself used for the deparser and validation logic internally.