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 support for "as-is" function calls for case-sensitive contexts (ClickHouse, BigQuery) #352

Closed
evilsneer opened this issue Sep 8, 2021 · 12 comments
Assignees
Labels
needs analysis I need to think about this!

Comments

@evilsneer
Copy link

It makes trouble when working with some sql-oriented dbs.

Example:

{:select [[[:domain :ref]]]}
produces
SELECT DOMAIN(ref)

@seancorfield
Copy link
Owner

Can you provide a concrete example of a database/JDBC driver combination where this causes a problem, so that I can set up a repro test case?

@evilsneer
Copy link
Author

sure
clickhouse with [ru.yandex.clickhouse/clickhouse-jdbc "0.3.1"]

@seancorfield
Copy link
Owner

Never heard of it. Looks like I can run it via Docker: https://hub.docker.com/r/yandex/clickhouse-server/

Can you provide a short Clojure script that, starting with an empty default instance of ClickHouse via that Docker container, creates a database, a table, and then does something that breaks specifically because of the uppercasing of the function?

That will give me a good testing framework so that I can compare behavior across some other DB/drivers and see if this is specific to ClickHouse and then I can look at providing some option for opting out of the default behavior somehow, in a way that won't break anything for users of any other system (this is not just about uppercasing function calls because the logic around that syntax is there to support other SQL syntax -- and this would also have to potentially affect every other context that HoneySQL currently treats as a function call).

You may just be better off using :raw to construct lowercase function calls for the (few?) cases where they actually break?

dev=> (sql/format {:select [[[:raw "domain(ref)"]]]})
["SELECT domain(ref)"]

@evilsneer
Copy link
Author

I have got sql for this

CREATE table fuzz ("ref" String) ENGINE = MergeTree() ORDER BY "ref";
INSERT INTO fuzz VALUES ('http://hello.com');

SELECT DOMAIN("ref") from fuzz - won't work with

DB::Exception: Unknown function DOMAIN. Maybe you meant: ['domain']: While processing DOMAIN(ref) (version 21.8.5.7 (official build))

expected

SELECT domain("ref") from fuzz

@seancorfield
Copy link
Owner

Much appreciated @evilsneer -- I'll take a look at this during my next OSS window (which probably won't be for a week or two, just to set expectations).

@seancorfield
Copy link
Owner

I'm still thinking about how best to tackle this since it is non-trivial.

@seancorfield
Copy link
Owner

BigQuery is another use case for this.

@seancorfield seancorfield changed the title Can uppercasing for functions calls be optional? Add support for "as-is" function calls for case-sensitive contexts (ClickHouse, BigQuery) Feb 2, 2022
@seancorfield
Copy link
Owner

seancorfield commented Feb 3, 2022

develop now supports function keywords that start with ' to produce literal names:

dev=> (sql/format {:select [[[:domain :ref]]]})
["SELECT DOMAIN(ref)"]
dev=> (sql/format {:select [[[:'domain :ref]]]})
["SELECT domain(ref)"]

Note: there is no literal symbol equivalent to this (although you could do (symbol "'domain")):

dev=> (sql/format {:select [[[(symbol "'domain") :ref]]]})
["SELECT domain(ref)"]

/cc @shay-rf

seancorfield added a commit that referenced this issue Feb 3, 2022
seancorfield added a commit that referenced this issue Feb 3, 2022
seancorfield added a commit that referenced this issue Feb 3, 2022
@seancorfield
Copy link
Owner

@evilsneer You posted a comment and seem to have deleted it -- was your question answered?

@evilsneer
Copy link
Author

Yes, i thought about some use cases with swapping sql backends below hsql. But at second thought think it’s not such useful, so differences of hsql representation btw backends is ok i guess

@evilsneer
Copy link
Author

evilsneer commented Feb 17, 2022

Hi again! Should it works already?

I have

[com.github.seancorfield/honeysql "2.2.861"]

and

(-> 
  (honey.sql.helpers/select :*) 
  (honey.sql.helpers/from :foo)
  (honey.sql.helpers/where [:'my-schema.SomeFunction :bar 0])
  (honey/format))
=> ["SELECT * FROM foo WHERE 'MY SCHEMA.SOMEFUNCTION(bar, ?)" 0]
(honey/format {:select [[[:'domain :ref]]]})
=> ["SELECT 'DOMAIN(ref)"]
(honey/format {:select [[[(symbol "'domain") :ref]]]})
=> ["SELECT 'DOMAIN(ref)"]

I see there is upcoming 2.2.next according to Changelog, any plans about release date?=)

@seancorfield
Copy link
Owner

It's available on develop via git deps. It has not yet been released. See https://github.com/seancorfield/honeysql/blob/develop/CHANGELOG.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs analysis I need to think about this!
Projects
None yet
Development

No branches or pull requests

2 participants