> - The column aliases returned by Calcite are in uppercase, while those > returned by PostGIS through the JDBC adapter are in lowercase.
I think this might be due to how the parser handles unquoted identifiers. In Calcite, as in Oracle, if I wrote ’select empno as x from emp’, the unquoted identifiers become EMPNO, X, EMP. Therefore the column in the result set will be called X, not x. The behavior is set via Lex.unquotedCasing. > For some reason, setting the conformance to LENIENT in the PostgisSqlDialect > is not sufficient. Not sure, but generally the dialects only come into play late in the query lifecycle, when the JDBC adapter is generating SQL. When the parser is parsing SQL, and the validator is validating an AST, there is no dialect in play yet. > should I aim at moving the TestContainers tests into the Calcite repository? I am torn on this. On one hand, I would like better testing for each of the dialects supported by the JDBC adapter (including Postgis). On the other, depending on containers adds to our dependencies and increases our flakiness. (Note the perennial problems in our Druid integration tests.) I have proposed https://issues.apache.org/jira/browse/CALCITE-5529 as a solution to this. You would run the direct tests (mainly RelToSqlConverterTest) against a server of that dialect (or a container), record the results in a requests-responses file, and commit that file to Calcite. Then anyone else would be able to run that dialect test offline, checking that Calcite (still) generates valid SQL for that dialect. This seems to me the best of both worlds - good test coverage without the slowness and flakiness. Julian > On Feb 8, 2024, at 12:14 PM, Bertil Chapuis <bchap...@gmail.com> wrote: > > Hello Everyone, > > Quick follow-up on the SQL dialect for PostGIS [1]. I've created a PR for the > new dialect and made a few additional modifications for the integration to > work with PostGIS through the JDBC adapter [2]. I also created an additional > repository to perform more advanced integration tests using TestContainers > [2]. I was pleasantly surprised to see all the spatial Quidem tests execute > seamlessly on PostGIS with only a few modifications made to the queries in > the sql/spatial.iq file [3]. > > Here are three questions on which I’d love to get some advice: > - The column aliases returned by Calcite are in uppercase, while those > returned by PostGIS through the JDBC adapter are in lowercase. I modified the > sql/spatial.iq file accordingly, but I’d love to find a better workaround. Is > there a good way to solve this issue at the level of the JDBC adapter or of > the dialect [4]? > - For some reason, setting the conformance to LENIENT in the > PostgisSqlDialect is not sufficient. The value is still DEFAULT at runtime, > and one must override the conformance level manually [5]. Is this a bug? > Also, should I introduce a dedicated conformence for POSTGIS or is it better > to use LENIENT? > - Finally, should I aim at moving the TestContainers tests into the Calcite > repository? I’m not sure it’s a good idea as the use of TestContainers is > limited in Calcite. > > Thanks a lot to those who already commented the PR. As it introduces >900 LoC > in areas I’m not familiar with, any feedback is welcome. > > Best, > > Bertil > > [1] https://issues.apache.org/jira/browse/CALCITE-6239 > [2] https://github.com/apache/calcite/pull/3668 > [3] https://github.com/bchapuis/calcite-postgis-tests > <https://github.com/bchapuis/calcite-postgis-tests/blob/f7d7df1dbff67d5beed2872f44f92b85ed18c18f/src/test/java/com/github/bchapuis/calcite_postgis_tests/QuidemPostgisTest.java#L88> > [4] > https://github.com/bchapuis/calcite-postgis-tests/blob/f7d7df1dbff67d5beed2872f44f92b85ed18c18f/src/test/java/com/github/bchapuis/calcite_postgis_tests/QuidemPostgisTest.java#L94 > [5] > https://github.com/bchapuis/calcite-postgis-tests/blob/f7d7df1dbff67d5beed2872f44f92b85ed18c18f/src/test/java/com/github/bchapuis/calcite_postgis_tests/QuidemPostgisTest.java#L102 > > >> On 17 Jan 2024, at 11:22, Bertil Chapuis <bchap...@gmail.com> wrote: >> >> Hello Everyone, >> >> Calcite implements support for spatial types (geometry, point, etc.) and >> spatial functions (ST_), and it can connect to PostGIS via a JdbcSchema. >> However, the Postgresql dialect does not currently handle spatial types and >> functions. As a result, Calcite tries to execute the spatial functions at >> the level of the JVM instead of pushing them down to postgis. >> >> As a result, the following query gets executed, but the type of the geom >> column is incorrect: >> SELECT id, geom FROM public.spatial_table >> >> The following query fails with a ClassCastException as Calcite tries to use >> the java implementation of the ST_SRID function: >> SELECT id, ST_SRID(geom) FROM public.spatial_table >> java.lang.ClassCastException: class org.postgresql.util.PGobject cannot be >> cast to class org.locationtech.jts.geom.Geometry >> (org.postgresql.util.PGobject and org.locationtech.jts.geom.Geometry are in >> unnamed module of loader 'app') >> >> In my current understanding, this issue could be addressed with a new >> PostgisSqlDialect that extends PostgresqlSqlDialect and adds support for >> spatial types and functions. Here is a tentative roadmap: >> - Add all the spatial functions to the SqlKind class >> - Create a PostgisSqlDialect class that extends PostgresqlSqlDialect >> - Add support for the spatial types (geometry) by overriding the getCastSpec >> method of the SqlDialect class >> - Add support for the spatial functions by overriding the supportsFunction >> method of the SqlDialect class >> - Add support for the spatial aggregate functions by overriding the >> supportsAggregateFunction method of the SqlDialect class >> >> Could someone confirm that overall this approach makes sense? If so, I will >> create a JIRA issue and submit pull requests. >> >> Something that I am not sure about, is how to test the dialect. In my >> understanding, when it comes to niche dialects such as PostGIS, Calcite >> relies on its users to report issues. Is that correct? >> >> Best, >> >> Bertil >
