Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1454

Allow custom implementations of SqlConformance

        Details

        • Type: Bug
        • Status: Closed
        • Priority: Major
        • Resolution: Fixed
        • Affects Version/s: None
        • Fix Version/s: 1.11.0
        • Component/s: None
        • Labels:
          None

          Description

          Allow custom implementations of SqlConformance. This would mean renaming enum SqlConformance to enum SqlConformanceEnum, and adding a new interface SqlConformance; also public abstract class SqlAbstractConformance implements SqlConformance to protect people who write custom implementations of the interface from changes in future.

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            julianhyde Julian Hyde added a comment -

            Maryann Xue, Can you please review my fix in https://github.com/julianhyde/calcite/tree/1454-custom-conformance?

            Show
            julianhyde Julian Hyde added a comment - Maryann Xue , Can you please review my fix in https://github.com/julianhyde/calcite/tree/1454-custom-conformance?
            Hide
            Permalink
            maryannxue Maryann Xue added a comment -

            All looks good to me, but one question: why don't we remove "allowBangEqual()" (and all related getters/setters) in SqlParser and replace it with "conformance().isBangEqualAllowed()"?
            Thank you very much for making this change, Julian Hyde!

            Show
            maryannxue Maryann Xue added a comment - All looks good to me, but one question: why don't we remove "allowBangEqual()" (and all related getters/setters) in SqlParser and replace it with "conformance().isBangEqualAllowed()"? Thank you very much for making this change, Julian Hyde !
            Hide
            Permalink
            jnadeau Jacques Nadeau added a comment -

            I'd go one step further and remove "bangEqual" all together. I really think we should avoid object identity checks everywhere in Calcite. As a library with many consumers, the reliance on object identity checks at various places makes things very difficult. I've know we've been bitten by this pattern multiple times. I haven't seen any analyses that show this is fixing a bottleneck.

            Show
            jnadeau Jacques Nadeau added a comment - I'd go one step further and remove "bangEqual" all together. I really think we should avoid object identity checks everywhere in Calcite. As a library with many consumers, the reliance on object identity checks at various places makes things very difficult. I've know we've been bitten by this pattern multiple times. I haven't seen any analyses that show this is fixing a bottleneck.
            Hide
            Permalink
            julianhyde Julian Hyde added a comment -

            Jacques Nadeau, Wrong tree. We're talking about SQL's != operator here.

            Show
            julianhyde Julian Hyde added a comment - Jacques Nadeau , Wrong tree. We're talking about SQL's != operator here.
            Hide
            Permalink
            jnadeau Jacques Nadeau added a comment -

            Hah!

            Show
            jnadeau Jacques Nadeau added a comment - Hah!
            Hide
            Permalink
            jnadeau Jacques Nadeau added a comment -

            Goes to show you what recent Calcite thing I was fighting with

            Show
            jnadeau Jacques Nadeau added a comment - Goes to show you what recent Calcite thing I was fighting with
            Hide
            Permalink
            julianhyde Julian Hyde added a comment -

            Maryann Xue, thanks for your review. I implemented your suggestion. Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/105bba1f.

            Show
            julianhyde Julian Hyde added a comment - Maryann Xue , thanks for your review. I implemented your suggestion. Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/105bba1f .
            Hide
            Permalink
            julianhyde Julian Hyde added a comment -

            Resolved in release 1.11.0 (2017-01-11).

            Show
            julianhyde Julian Hyde added a comment - Resolved in release 1.11.0 (2017-01-11).

              People

              • Assignee:
                julianhyde Julian Hyde
                Reporter:
                julianhyde Julian Hyde
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development