Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix antlr dep partiql-lang #1534

Merged
merged 2 commits into from
Aug 2, 2024
Merged

Fix antlr dep partiql-lang #1534

merged 2 commits into from
Aug 2, 2024

Conversation

alancai98
Copy link
Member

@alancai98 alancai98 commented Aug 2, 2024

Relevant Issues

  • None

Description

  • Tries fixing the partiql-lang dependency on partiql-parser
  • Since the ANTLR shading change (Shade antlr dependency for partiql-parser and partiql-lang #1439), any call to the PartiQLParserBuilder.standard() from the Maven release of 0.14.6 would result in a NoSuchMethodError when attempting to parse. This was due to a partiql-lang using its own ANTLR dependency rather than partiql-parser's shaded ANTLR dependency.
    • It was not detected sooner in the normal gradle build because we would previously only generate the shaded ANTLR dependency when publishing the jars to Maven.
    • We should have been using this configuration provided by the shadow plugin for multi-project builds.
  • Simple reproduction case for previous failure w/ v0.14.6
// in build.gradle.kts
dependencies {
    implementation("org.partiql:partiql-lang-kotlin:0.14.6")
}
// in Main.kt
import org.partiql.lang.syntax.PartiQLParserBuilder

fun main(args: Array<String>) {
    val input = "SELECT 1 FROM <<>>"
    val parser = PartiQLParserBuilder.standard().build()
    println(parser.parseAstStatement(input))
}

above results in the following error:

Exception in thread "main" org.partiql.lang.syntax.ParserException: Unhandled exception.
	Evaluator Error: at line <UNKNOWN>, column <UNKNOWN>: internal error, <UNKNOWN> : <UNKNOWN>

	at org.partiql.lang.syntax.impl.PartiQLPigParser.parseAstStatement(PartiQLPigParser.kt:74)
	at MainKt.main(Main.kt:6)
Caused by: java.lang.NoSuchMethodError: 'void org.partiql.parser.antlr.PartiQLTokens.<init>(org.partiql.lang.thirdparty.antlr.v4.runtime.CharStream)'
	at org.partiql.lang.syntax.impl.PartiQLPigParser.createTokenStream$partiql_lang(PartiQLPigParser.kt:111)
	at org.partiql.lang.syntax.impl.PartiQLPigParser.parseQuery$partiql_lang(PartiQLPigParser.kt:95)
	at org.partiql.lang.syntax.impl.PartiQLPigParser.parseQuery(PartiQLPigParser.kt:85)
	at org.partiql.lang.syntax.impl.PartiQLPigParser.parseAstStatement(PartiQLPigParser.kt:58)
	... 1 more

Note that when using the PartiQLParserBuilder.experimental... rather than PartiQLParserBuilder.standard..., the query parses without errors.

Other Information

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

github-actions bot commented Aug 2, 2024

Conformance comparison report

Base (6c314d2) 1075fbd +/-
% Passing 92.54% 92.54% 0.00%
✅ Passing 5384 5384 0
❌ Failing 434 434 0
🔶 Ignored 0 0 0
Total Tests 5818 5818 0

Number passing in both: 5384

Number failing in both: 434

Number passing in Base (6c314d2) but now fail: 0

Number failing in Base (6c314d2) but now pass: 0

@alancai98
Copy link
Member Author

Local testing within partiql-lang-kotlin

  • gradle clean and build partiql-lang-kotlin -- all previous tests passed
  • publishToMavenLocal

In a sample project

  • Created a simple gradle project w/ org.partiql:partiql-lang-kotlin:0.14.7 dependency
// In build.gradle.kts
repositories {
    mavenCentral()
    mavenLocal()
}

dependencies {
    implementation("org.partiql:partiql-lang-kotlin:0.14.7")
}

and in

// Main.kt
import org.partiql.lang.syntax.PartiQLParserBuilder

fun main(args: Array<String>) {
    val input = "SELECT 1 FROM <<>>"
    val parser = PartiQLParserBuilder.standard().build()
    println(parser.parseAstStatement(input))
}

Verified that the reproduction case from PR description now works without an error.

Other validation

  • Inspected the locally-published partiql-lang-kotlin-0.14.7.jar and checked that the ANTLR dependencies used in partiql-lang come from partiql-parser's shadowed ANTLR dependency under org.partiql.parser.thirdparty.antlr

Copy link
Member

@johnedquinn johnedquinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally. Thanks for tackling this issue!

@alancai98 alancai98 merged commit f1d785f into prep-0_14_7 Aug 2, 2024
10 checks passed
@alancai98 alancai98 deleted the fix-antlr-dep-lang branch August 2, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants