[SPARK-27016][SQL][BUILD] Treat all antlr warnings as errors while generating parser from the sql grammar file.

[dilipbiswal]

What changes were proposed in this pull request?

Use the maven plugin option treatWarningsAsErrors to make sure the warnings are treated as errors while generating the parser file. In the absence of it, we may inadvertently introducing problems while making grammar changes. Please refer to PR-23897 to know more about the context.

How was this patch tested?

We can use two ways to build Spark 1) sbt 2) Maven
This PR, we made a change to configure the maven antlr plugin to include a parameter that makes antlr4 report error on warning. However, when spark is built using sbt, we use the sbt antlr plugin which does not allow us to pass this additional compilation flag. More info on sbt-antlr plugin can be found at link
In summary, this fix only applicable when we use maven to build.

[dilipbiswal]

I am expecting the build to fail due to an existing issue with the grammar file. After 23897 is merged, the error should go away.

[dilipbiswal]

@srowen Hi Sean, need your help here. In my dev environment, when i use maven to build, i get the expected result and the build fails with the following error :

INFO] 
[INFO] --- antlr4-maven-plugin:4.7.1:antlr4 (default) @ spark-catalyst_2.12 ---
[INFO] ANTLR 4: Processing source directory /Users/dbiswal/mygit/apache/spark/sql/catalyst/src/main/antlr4
[INFO] Processing grammar: org/apache/spark/sql/catalyst/parser/SqlBase.g4
[WARNING] warning(125): org/apache/spark/sql/catalyst/parser/SqlBase.g4:784:90: implicit definition of token SCHEMA in parser
[WARNING] /Users/dbiswal/mygit/apache/spark/org/apache/spark/sql/catalyst/parser/SqlBase.g4 [784:90]: implicit definition of token SCHEMA in parser
[ERROR] error(10):  warning treated as error
[INFO] ------------------------------------------------------------------------

However, from jenkins, it uses sbt and the build seems to skip pass this error/warning. Would you know what may be happening and how to fix this ? Appreciate your help.

[srowen]

Hm, I'm not sure. SBT 'reads' the pom file via a plugin, but I think the translation is imperfect. I'm still surprised. Sometimes we've worked around it by duplicating the config and hacking it into the SBT build. I'm not sure how to do it here.

We use Maven (also) in the test jobs that aren't PR builders, so this would still get caught eventually, and I presume is a pretty rare thing to update the schemas. We might just live with it unless someone has a better idea.

(Also, I wonder if we should make the PR builders use Maven, as it's the build of record really)

[dilipbiswal]

@srowen Thanks a lot for quick response.

Also, I wonder if we should make the PR builders use Maven, as it's the build of record really.

IMHO we should especially if sbt swallows exceptions/errors like this :-)

[dilipbiswal]

@srowen Update : I looked around. In my understanding, we seem to use the sbt-antlr plugin to generate the parser when compiling through sbt (SparkBuild.scala). Unfortunately, there is no (at least i couldn't find) "fail build on warning" option that is exposed through the sbt-antlr plugin
Here is link to the plugin source.

[maropu]

cc: @shaneknapp @gatorsmile

[SparkQA]

Test build #102876 has finished for PR 23925 at commit df0f530.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[dongjoon-hyun]

@dilipbiswal . Could you update the PR description Verified this after applying PR part according to the latest #23925 (comment) ?

[dilipbiswal]

@dongjoon-hyun Sure.. Let me do it.

[SparkQA]

Test build #4586 has started for PR 23925 at commit df0f530.

[dilipbiswal]

retest this please

[SparkQA]

Test build #102942 has finished for PR 23925 at commit df0f530.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

Merged to master

[dilipbiswal]

Thanks a lot @srowen.
FYI. I have opened an issue against the sbt antlr plugin here.