On Thu, Apr 30, 2020 at 11:26 PM rahul patwari <[email protected]> wrote:
> Hi, > > A JIRA ticket is raised to track this bug: BEAM-8307 > <https://issues.apache.org/jira/browse/BEAM-8307> > > I have raised a PR: https://github.com/apache/beam/pull/11581 to fix the > issue. > > This PR takes care of using BeamSql with JdbcIO. > I would be interested to contribute if any other IOs supported by Beam > requires a similar fix like the one in this PR so that they can be used > with BeamSql. > > What could be a cleaner approach, in general, to handle this for all the > IOs? > > Also, what can be done to support BeamSql with User-Defined Logical Types? > Should they be converted to one of the Beam SQL Types[1] before applying > SqlTransform.query()? > Should we expose an interface to provide Calcite RelDataType Mapping for > User-Defined Logical Types? > First I want to say I don't think the JdbcIO types you're adding fixes for in https://github.com/apache/beam/pull/11581 should be considered "user-defined logical types". These are all well known types that we *should* have standard logical types for in schemas.logicaltypes, with a URN identifier. Then both JDBC and SQL can reference those same logical types and there's no need to add a mapping in CalciteUtils. This is what +Robin Qiu <[email protected]> is doing for DATE in [1]. As far as user-defined logical types in general, personally I don't think SqlTransform should accept them. I brought this up at the end of a recent message [2]. I'll copy the relevant part here so you don't have to wade through it :) > ... I'm against the idea of > implicitly stripping away logical types like this in schema-aware > transforms. A logical type indicates that the value has some additional > meaning beyond just the base type (and maybe a restricted range), and I > don't think transforms should be able to ignore that meaning unless the > user explicitly allows it, or first converts to the base type themselves. If users really want to process custom logical types with SQL, they could just convert them to their base types first. If this is common, we could add an option to SqlTransform to have it do this automatically for unrecognized types. [1] https://github.com/apache/beam/pull/11272 [2] https://lists.apache.org/thread.html/r2e05355b74fb5b8149af78ade1e3539ec08371a9a4b2b9e45737e6be%40%3Cdev.beam.apache.org%3E > Let me know your thoughts. > > [1]: > https://github.com/apache/beam/blob/b8aa8486f336df6fc9cf581f29040194edad3b87/sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/utils/CalciteUtils.java#L43 > > Regards, > Rahul >
