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

Add more parse tests from partiql-lang-kotlin; add omitted tests list #11

Merged
merged 2 commits into from
May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ The test data is partitioned into the following directories:
Subdirectories:
- `pass/parser` - statements that can parse without error
- `fail/parser` - statements that give an error when parsed
- `fail/semantic` - statements that give an error at some stage between parsing and evaluation
- `pass/eval` - statements that can be evaluated successfully and return the same result as the expected result
- `fail/eval` - statements that throw an error during evaluation

Expand Down
71 changes: 71 additions & 0 deletions partiql-lang-kotlin-omitted-tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
The following lists out the tests from `partiql-lang-kotlin`'s parse tests that are not part of `partiql-tests`:

Tests excluded:
- Custom type cast
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L631-L644
- [SqlParserCastTests.kt](https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserCastTests.kt)
- [SqlParserCustomTypeCatalogTests.kt](https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserCustomTypeCatalogTests.kt)
- ORDER BY tests testing defaults for sort spec and null spec
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L1793-L1888
- dependent on when parsed ast checking is added or up to the implementation to test via correspondence (see comment here https://github.com/partiql/partiql-tests/pull/11/files#r865515909)
- alternatively, this may be better tested in the evaluator
- Many nested NOT regression test
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L4239-L4271
- performance test shouldn't be included in parse tests
alancai98 marked this conversation as resolved.
Show resolved Hide resolved
- LET clause parsing (not part of spec formally yet)
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L4066-L4128
- `expectedAsForLet` https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L431-L443
- `expectedIdentForAliasLet` https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L473-L485
- `idIsNotStringLiteral` -- logic tested elsewhere
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L768-L780
- DML, DDL tests (DML, DDL not defined in spec yet)
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L2129-L3855
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L3857-L3939
- `unexpectedKeywordUpdateInSelectList` - https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L132-L142
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1291-L1457
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1980-L2662
- Stored procedure calls (EXEC) not in spec yet
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L2760-L2846
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L4179-L4237
- semicolon semantic tests
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1860-L1886
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L4025-L4064

Unsure if should be included:
- `expectedCastAsIntArity`, `expectedCastAsRealArity` -- type supplied with parameter; not sure is parse error
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L172-L204
- `expectedUnsupportedLiteralsGroupBy` -- not part of the PartiQL spec; SQL92 says group by must use a column name
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L417-L429
- also for `groupByOrdinal`, `groupByOutOfBoundsOrdinal`, `groupByBadOrdinal`, `groupByStringConstantOrdinal` https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L953-L1007
- `idIsNotGroupMissing` -- type surrounded by parens
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L782-L794
- `castToVarCharToTooBigLength`, `castToDecimalToTooBigLength_1`, `castToDecimalToTooBigLength_2`, `castToNumericToTooBigLength_1`, `castToNumericToTooBigLength_2` -- not quite a parse error; more so a semantic error
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L220-L288
- Similarly for `castNegativeArg`
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L883-L895
- `offsetBeforeLimit` -- not part of spec; postgresql allows; kotlin impl+mysql+sqlite don't allow; included as test for now
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1221-L1233
- date_add/date_diff tests - date_add and date_diff aren't part of specs
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserTest.kt#L957-L1036
- https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1743-L1858
- `countExpressionStar`
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1966-L1978
- `*` and path `*` with other select list items
Copy link
Contributor

Choose a reason for hiding this comment

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

Is that because you consider them as a semantic error?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unsure it would be an error to begin with. The Kotlin implementation doesn't allow using * in a select list with other items.

I can't find a section of the PartiQL or SQL92 spec that forbids this. PostgreSQL + SQLite allow those cases. MySQL seems to only allow * as the first item in the select list (can have other columns following the *).

For now, leaving in the "unsure" section, but open to moving if there's something I missed from the spec.

https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1903-L1936
Comment on lines +34 to +54
Copy link
Member Author

@alancai98 alancai98 May 3, 2022

Choose a reason for hiding this comment

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

Rendered file.

Would be good to get another pair of eyes on this file, in particular the "Unsure if should be included" section for tests I was unsure to include (i.e. tests not part of the PartiQL/SQL spec but are defined as errors by the Kotlin implementation).

The "Tests excluded" (L3-33) may also be relevant to look at. These were tests that were

  1. not in spec but likely will be sometime in the future such as DML, DDL, EXEC, LET, etc
  2. tests not really parser related (e.g. performance, default parsed AST, custom type)

Copy link
Member Author

Choose a reason for hiding this comment

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

Rendered file link updated following 26a5720 commit.

- `callTrimSpecificationMissingFrom` -- PostgreSQL supports; unsure of utility of creating this fail test
- https://github.com/~~partiql~~/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L683-L696
- More details provided by Josh in this comment on the ambiguity https://github.com/partiql/partiql-tests/pull/11/files#r865505304
Repeated tests (tests repeated at some other place):
- `expectedExpectedTypeName`
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L78-L90
- `likeWrongOrderOfArgs`, `likeMissingEscapeValue`, `likeMissingPattern`
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1487-L1527
- `nullIsNullIonLiteral`, `idIsStringLiteral`
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1585-L1611
- `selectWithFromAtAndAs`
https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L1627-L1639

To be included:
- DATE and TIME tests from [SqlParserDateTimeTests.kt](https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserDateTimeTests.kt)
and [ParserErrorsTest.kt](https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/errors/ParserErrorsTest.kt#L2848-L3099)
- Operator precedence tests from [SqlParserPrecedenceTest.kt](https://github.com/partiql/partiql-lang-kotlin/blob/34625c68dbcbaf7b8ae60df7a4cf65c60b2a3b79/lang/test/org/partiql/lang/syntax/SqlParserPrecedenceTest.kt)
58 changes: 58 additions & 0 deletions partiql-test-data/fail/parser/primitives/call.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
substring::[
sql92::[
parse::{
name: "SUBSTRING sql92 syntax missing left paren",
statement: "SELECT SUBSTRING FROM 'asdf' FOR 1) FROM foo",
assert: {
result: ParseError
},
},
parse::{
name: "SUBSTRING sql92 syntax missing FROM or comma",
statement: "SELECT SUBSTRING('str' 1) FROM foo",
assert: {
result: ParseError
},
},
parse::{
name: "SUBSTRING sql92 syntax without length and missing right paren",
statement: "SELECT SUBSTRING('str' FROM 1 FROM foo",
assert: {
result: ParseError
},
},
parse::{
name: "SUBSTRING sql92 syntax with length missing right paren",
statement: "SELECT SUBSTRING('str' FROM 1 FOR 1 FROM foo",
assert: {
result: ParseError
},
}
],
partiql::[
parse::{
name: "SUBSTRING without length missing right paren",
statement: "SELECT SUBSTRING('str', 1 FROM foo",
assert: {
result: ParseError
},
},
parse::{
name: "SUBSTRING missing right paren",
statement: "SELECT SUBSTRING('str', 1, 1 FROM foo",
assert: {
result: ParseError
},
}
]
]

trim::[
parse::{
name: "TRIM no left paren",
statement: "TRIM ' ')",
assert: {
result: ParseError
},
},
]
31 changes: 31 additions & 0 deletions partiql-test-data/fail/parser/primitives/case.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
parse::{
name: "CASE expected WHEN clause",
statement: "CASE name ELSE 1 END",
assert: {
result: ParseError
},
}

parse::{
name: "CASE only with END",
statement: "CASE END",
assert: {
result: ParseError
},
}

parse::{
name: "searched CASE no when with ELSE",
statement: "CASE ELSE 1 END",
assert: {
result: ParseError
},
}

parse::{
name: "simple CASE no when with ELSE",
statement: "CASE name ELSE 1 END",
assert: {
result: ParseError
},
}
15 changes: 15 additions & 0 deletions partiql-test-data/fail/parser/primitives/cast.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
parse::{
name: "CAST expected left paren",
statement: "CAST 5 as integer",
assert: {
result: ParseError
},
}

parse::{
name: "CAST given keyword as type arg",
statement: "CAST(5 AS SELECT)",
assert: {
result: ParseError
},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
parse::{
name: "VALUES constructor expect left paren",
statement: "VALUES 1,2)",
assert: {
result: ParseError
},
}
31 changes: 31 additions & 0 deletions partiql-test-data/fail/parser/primitives/expressions.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
parse::{
name: "missing right paren",
statement: "(1 + 2",
assert: {
result: ParseError
},
}

parse::{
name: "missing operation between literals",
statement: "5 5",
assert: {
result: ParseError
},
}

parse::{
name: "semicolon inside expression",
statement: "(1;)",
assert: {
result: ParseError
},
}

parse::{
name: "VALUE as top-level expression",
statement: "VALUE 1",
assert: {
result: ParseError
},
}
23 changes: 23 additions & 0 deletions partiql-test-data/fail/parser/primitives/operators/at-operator.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
parse::{
name: "at operator without identifier",
statement: "@",
assert: {
result: ParseError
},
}

parse::{
name: "at operator on non identifier",
statement: "@(a)",
assert: {
result: ParseError
},
}

parse::{
name: "double at operator on identifier",
statement: "@ @a",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the space between the @ intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's intentional. Test taken from here. Would another test without the space be helpful to add?

assert: {
result: ParseError
},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
parse::{
name: "BETWEEN operator missing AND",
statement: "5 BETWEEN 1 10",
assert: {
result: ParseError
},
}
23 changes: 23 additions & 0 deletions partiql-test-data/fail/parser/primitives/operators/is-operator.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
parse::{
alancai98 marked this conversation as resolved.
Show resolved Hide resolved
name: "IS NOT operator expects type name",
statement: "NULL IS NOT `null`",
assert: {
result: ParseError
},
}

parse::{
name: "IS operator parens around type name",
statement: "a IS (MISSING)",
assert: {
result: ParseError
},
}

parse::{
name: "IS NOT operator parens around type name",
statement: "a IS NOT (MISSING)",
assert: {
result: ParseError
},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
parse::{
name: "LIKE wrong order of args",
statement: "SELECT a, b FROM data WHERE LIKE a b",
assert: {
result: ParseError
},
}

parse::{
name: "LIKE expected expression following ESCAPE",
statement: "SELECT a, b FROM data WHERE a LIKE b ESCAPE",
assert: {
result: ParseError
},
}

parse::{
name: "LIKE missing rhs argument",
statement: "SELECT a, b FROM data WHERE a LIKE",
assert: {
result: ParseError
},
}

parse::{
name: "LIKE col name LIKE col name ESCAPE typo",
statement: "SELECT a, b FROM data WHERE a LIKE b ECSAPE '\\'",
assert: {
result: ParseError
},
}

parse::{
name: "LIKE ESCAPE before LIKE",
statement: "SELECT a, b FROM data WHERE ESCAPE '\\' a LIKE b",
assert: {
result: ParseError
},
}

parse::{
name: "LIKE ESCAPE as second argument",
statement: "SELECT a, b FROM data WHERE a LIKE ESCAPE '\\' b",
assert: {
result: ParseError
},
}
15 changes: 15 additions & 0 deletions partiql-test-data/fail/parser/primitives/path-expression.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
parse::{
name: "invalid path component too many dots",
statement: "x...a",
assert: {
result: ParseError
},
}

parse::{
name: "invalid path component keyword in path",
statement: '''SELECT foo.id, foo.table FROM `[{id: 1, table: "foos"}]` AS foo''',
assert: {
result: ParseError
},
}
7 changes: 7 additions & 0 deletions partiql-test-data/fail/parser/query/pivot.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
parse::{
name: "PIVOT no AT",
statement: "PIVOT v FROM data",
assert: {
result: ParseError
},
}
Loading