Support MySQL GRANT SENSITIVE_VARIABLES_OBSERVER

[kgalieva]

Adding support for GRANT SENSITIVE_VARIABLES_OBSERVER for MySQL
https://dev.mysql.com/doc/refman/8.4/en/privileges-provided.html

[kaby76]

@mike-lischke Is this change correct to the Oracle grammar? I just updated the grammar to the lastest from the vscode extension. #4408

[mike-lischke]

This PR should be rejected. It adds a dynamic privilege to the grammar. The grant statement not only allows to specify predefined privileges, but also simple identifiers. Hence you can use input like grant SENSITIVE_VARIABLES_OBSERVER to [email protected] and does a runtime lookup for the given priv name.

The upstream fix is also wrong btw. @kgalieva. And, very important, syntax which is supported only starting at a specific MySQL version should be guarded using a semantic predicate. You can see that everywhere in the MySQL grammar.

[mike-lischke]

Side note: this PR is not for the Oracle grammar, but the one from Positive Technologies. I'm not sure this repository should keep multiple MySQL grammars.

[kaby76]

Side note: this PR is not for the Oracle grammar, but the one from Positive Technologies. I'm not sure this repository should keep multiple MySQL grammars.

Sorry, I see that now. I only saw that a test was modified over in the Oracle grammar, and it parses fine.

$ ./bin/Debug/net8.0/Test.exe -input 'GRANT SENSITIVE_VARIABLES_OBSERVER ON *.* TO `admin`@`%`;' -tree
(queries (query (simpleStatement (accountManagementStatement (grantStatement GRANT (roleOrPrivilegesList (roleOrPrivilege (roleIdentifierOrText (roleIdentifier (pureIdentifier SENSITIVE_VARIABLES_OBSERVER))))) ON (grantIdentifier * . *) TO (grantTargetList (userList (user (userIdentifierOrText (textOrIdentifier (identifier (pureIdentifier `admin`))) (userVariable @ (textOrIdentifier (identifier (pureIdentifier `%`))))))))))) ;) <EOF>)
CSharp 0 string0 success 0.103722
Total Time: 0.2760364
02/13-06:20:05 ~/issues/g4-current/sql/mysql/Oracle/Generated-CSharp

I agree. It might be time to archive the PT grammar, or just remove it.

[mike-lischke]

@teverett @KvanTTT What do you think? Shall we remove the PT MySQL grammar, in favor of the Oracle grammar? I know the PT grammar also supports MySQL 5.6 and 5.7, but these versions are EOL already and the PT grammar is only partially correct. If people really want support for these versions, they can download an earlier version of the current Oracle grammar from https://github.com/mysql/mysql-workbench/blob/8.0/library/parsers/grammars/.

[KvanTTT]

I'm not sure, because other users still use these grammars and even make pull requests. For instance, the latest was on Jan 16: #4383

Please ask them at first (@ani-sha) and create a related topic to make it noticable.

[mike-lischke]

I have a conflict of interests here, as my grammar would prevail, so maybe someone else is the better choice to drive this process. This PR here as well as the one you mentioned @KvanTTT are both wrong (I explained that above).

All I can do here is to suggest to remove the PT grammar, because it is not correct. It doesn't matter that someone still files PRs for it (it shows only the grammar needs changes to be correct). What happens instead it that PRs like this and #4383 add things that don't belong in this grammar. It's a MySQL grammar, not a TokuDB or Azure grammar. So with every such PR the grammar gets more and more incorrect, not to mention that it is way behind the actual MySQL time line (we are already working on 9.x).

[teverett]

I have no concerns with keeping the PT grammar. If it's wrong, perhaps it could be corrected?

[KvanTTT]

It doesn't matter that someone still files PRs for it (it shows only the grammar needs changes to be correct).

Well, but I think it matters.

I have no objection regarding removing, but please create an issue regarding the topic to notify users explicetly and get more other opinions. Unfortunately, legacy is not fun, but it's an important part of community support.

[ani-sha]

Hi @KvanTTT, thanks for tagging me in this discussion. I would like to highlight that Debezium has been using the Positive Technologies MySQL grammar since at least 2017. To keep up with the evolving MySQL syntax, our maintainers (and community members) actively contribute updates to the upstream PT grammar whenever we make any changes in our grammar.

It doesn't matter that someone still files PRs for it (it shows only the grammar needs changes to be correct). What happens instead it that PRs like this and #4383 add things that don't belong in this grammar. It's a MySQL grammar, not a TokuDB or Azure grammar. So with every such PR the grammar gets more and more incorrect.

In the past, we haven’t received feedback on our PRs regarding these concerns. We didn’t anticipate that the TokuDB or Azure grammar PRs would be merged here, but since they were, we contributed to ensure support for all MySQL grammar variations. That said, we are open to refining our contributions and providing specific semantic predicates where needed.

Please create an issue regarding the topic to notify users explicetly and get more other opinions.

That would be helpful. I will need to discuss these changes with my team.

[mike-lischke]

Please note: it was not my intention to use this issue to decide about the future of the PT grammar. All I wanted was to get a general yes/no input from important people in this process.

Of course there must be a good deprecation path etc. to phase out the grammar. Ideally one of the owners of it should take the stab and start this process.

[teverett]

@KvanTTT ok to merge this?

[teverett]

@ani-sha @KvanTTT are we waiting on feedback from @ani-sha before merging?

[ani-sha]

@teverett, please keep me informed about the deprecation process for Positive Technologies MySQL grammar. Once your deprecation process starts, my team has decided to transition to Oracle MySQL grammar, but we need to factor in your timeline. If anything occurs, please tag me so we can stay updated with the latest information.

Also, I have a question: If you plan to remove PT grammar, what about MariaDB grammar, which is built on top of PT grammar?

[mike-lischke]

@ani-sha The MariaDB is not based on any MySQL grammar, AFAIK. It's an own grammar defining everything from scratch.

[ani-sha]

@mike-lischke Well, I don't think it was created entirely from scratch. If you compare the MariaDB grammar with the PT grammar, it seems to be based on that. @Naros, isn't that right?

[mike-lischke]

@ani-sha It's probably copied, but it does not depend on the PT grammar, so removing that grammar has no impact on the MariaDB grammar.

@ALL I took the freedom and created #4429 to formally start the deprecation process. Please add your thoughts about my proposal there.

[kaby76]

In a scenario where MariaDB reuses the MySQL grammar, the Trash tester would require a few changes. In a .g4, the import statement does not contain any path information (it doesn't even have ".g4" in the grammar file name), so path info is placed on the Antlr tool command-line. The tester, which works across all OSes and all targets/ports, uses a "config file" with the desc.xml. It would require a change to support import path information, but it's an easy change. (What is more difficult to understanding how Antlr "import" works.)

[mike-lischke]

I explained the current import process in detail in my feature issue to extend it. This is, however, not related to this issue here.

[Naros]

@Naros, isn't that right?

Hi @ani-sha, it was, particularly given that up to that point, we had been contributing changes for both MySQL and MariaDB into the MySQL grammar for PT for quite some time based on the feedback we received from the maintainers here.

As for these PR changes, if the maintainers believe that the rules aren't correct, we should at least take their suggestions back to our code base and test them. If those suggestions pass, we should adjust not only our local grammar but also this PR so that the code remains aligned, at least until the future of the PT grammar is decided.

As to the proposal for deprecation, I'll leave my opinions in the #4429 issue.

[kaby76]

we had been contributing changes for both MySQL and MariaDB into the MySQL grammar for PT for quite some time based on the feedback we received from the maintainers here.

It sounds like the mariadb grammar is a copy of the PT grammar. I'll take a look to see what a grammar diff shows and create an Issue to track fixing the mariadb grammar.

Antlr4 import has always existed, but it wan't possible to do cross-grammar import in this repo because neither the old Maven tester nor the Trash tester supported cross-grammar import. But, I'm implementing this now for the Trash tester, and I have a grammar for TrapC that uses it. This should help reduce grammar forking.

[Naros]

It sounds like the mariadb grammar is a copy of the PT grammar. I'll take a look to see what a grammar diff shows and create an Issue to track fixing the mariadb grammar.

Just to be clear @kaby76, we've been using the MariaDB grammar since June 2024. Prior to that, if it was MariaDB-specific, we were adding it to the MySQL grammar locally, and in some cases contributing those upstream to MySQL. So I wouldn't be surprised if you see some changes in the MySQL PT grammar with comments that its MariaDB-specific between Jan 2023 and Jun 2024.

But since June 2024, for MariaDB-specific grammar we've been opening PRs against the MariaDB grammar only.