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.

Sign up for GitHub

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

Jump to bottom

Failure to parse DECIMAL(<scale>, <precision>) #228

Closed
dlurton opened this issue Mar 9, 2020 · 0 comments · Fixed by #230
Closed

Failure to parse DECIMAL(<scale>, <precision>) #228

dlurton opened this issue Mar 9, 2020 · 0 comments · Fixed by #230
Labels
bug Something isn't working

Comments

@dlurton
Copy link
Member

dlurton commented Mar 9, 2020 

PartiQL> cast(foo as decimal(2, 2))
   | !!
org.partiql.lang.syntax.ParserException: Apparently DataType needs multiple lengths, sizes, etc
	Parser Error: at line 1, column 13: Internal error - malformed parse tree detected, <UNKNOWN> : <UNKNOWN>

	at org.partiql.lang.syntax.SqlParser$ParseNode.errMalformedParseTree(SqlParser.kt:160)
	at org.partiql.lang.syntax.SqlParser.toDataType(SqlParser.kt:722)
	at org.partiql.lang.syntax.SqlParser.toExprNode(SqlParser.kt:305)
	at org.partiql.lang.syntax.SqlParser.parseExprNode(SqlParser.kt:1919)
	at org.partiql.cli.Repl$parsePartiQLWithFilters$1.invoke(Repl.kt:267)
	at org.partiql.cli.Repl$parsePartiQLWithFilters$1.invoke(Repl.kt:106)
	at org.partiql.cli.Repl$executeTemplate$totalMs$1.invoke(Repl.kt:212)
	at org.partiql.cli.Repl$executeTemplate$totalMs$1.invoke(Repl.kt:106)
	at org.partiql.cli.Timer$DefaultImpls.timeIt(Repl.kt:96)
	at org.partiql.cli.Repl$1.timeIt(Repl.kt:112)
	at org.partiql.cli.Repl.executeTemplate(Repl.kt:211)
	at org.partiql.cli.Repl.parsePartiQLWithFilters(Repl.kt:265)
	at org.partiql.cli.Repl.run(Repl.kt:316)
	at org.partiql.cli.Main.runRepl(main.kt:155)
	at org.partiql.cli.Main.main(main.kt:140)
ERROR!

Note that cast(foo as decimal(1)) works without issue however.

Tasks outlined by this issue:

  1. Update SqlParser to parse multiple arguments for DECIMAL and NUMERIC when they are used with 2 arguments. (Also verify that these are the only two types that include more than one parameter.)
  2. Add tests to SqlParserTests for those types when used with more than one argument.
@dlurton dlurton changed the title Failure to parse DECIMAL(<scale>, <precision>) with precision Failure to parse DECIMAL(<scale>, <precision>) Mar 9, 2020
@jonwilsdon jonwilsdon added the bug Something isn't working label Mar 16, 2020
dlurton added a commit that referenced this issue Mar 24, 2020
@dlurton
Fix #228 by removing invalid sanity check.
6e8ac27 
This sanity check threw an exception from SqlParser whenever a data type
reference that included more than one argument was encountered.  The
check appears to have been completely erroneous from the start and on
removivng it, no other modifications were needed.

This commit also adds a few tests to SqlParserTests to cover this
scenario and prevent regressions.
@dlurton dlurton mentioned this issue Mar 24, 2020
Merged
dlurton added a commit that referenced this issue Mar 25, 2020
@dlurton
Fix #228 by removing invalid sanity check. (#230)
74a270b 
This sanity check threw an exception from SqlParser whenever a data type
reference that included more than one argument was encountered.  The
check appears to have been completely erroneous from the start and on
removing it, no other modifications were needed.  However, note that 
at this time we do not actually do anything with those arguments.  At this
time their presence is simply tolerated to improve SQL-92 
compatibility.  See #231 for one way we could use these arguments.

This commit also adds a few tests to SqlParserTests to cover this
scenario and prevent regressions.
dlurton added a commit that referenced this issue Mar 25, 2020
@dlurton
Fix #228 by removing invalid sanity check. (#230)
91af4a5 
This sanity check threw an exception from SqlParser whenever a data type
reference that included more than one argument was encountered.  The
check appears to have been completely erroneous from the start and on
removing it, no other modifications were needed.  However, note that 
at this time we do not actually do anything with those arguments.  At this
time their presence is simply tolerated to improve SQL-92 
compatibility.  See #231 for one way we could use these arguments.

This commit also adds a few tests to SqlParserTests to cover this
scenario and prevent regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #228 by removing invalid sanity check. partiql/partiql-lang-kotlin
2 participants
@dlurton @jonwilsdon