Does java.jdbc depend on keywords that start with digits?

181 views
Skip to first unread message

Andy Fingerhut

unread,
Oct 26, 2013, 12:37:50 AM10/26/13
to cloju...@googlegroups.com, seanco...@gmail.com
After CLJ-1252's patch was committed today, java.jdbc fails tests with 1.6.0-master-SNAPSHOT that seem to rely upon keywords that start with digits, e.g :1

core.logic and core.match have such tests, too, but they seem to be easy to change not to use digits as the first character of any keywords.  java.jdbc seems like it might be more fundamental in its assumption that digits at the beginning of keywords are OK to use.

Andy

Andy Fingerhut

unread,
Oct 26, 2013, 12:46:35 AM10/26/13
to cloju...@googlegroups.com
Correction: core.logic and core.cache are the two other libraries with failures due to CLJ-1252's patch, but are easy to fix if CLJ-1252's patch is kept.  core.match is fine either way.

Andy

Alex Miller

unread,
Oct 26, 2013, 9:13:45 AM10/26/13
to cloju...@googlegroups.com
I was wondering what would pop out from that change. The change brings Clojure up to the reader definition that has always been there (http://clojure.org/reader) and was the incorrectly executed intent of the prior code. We should have a 1.6.0-alpha1 out soon that people can work against.

For the java.jdbc failures, it looks like it wouldn't be too hard to fix the tests so they succeed, but if I'm reading the code right, the keyword numbers are coming back as the result insert-records (https://github.com/clojure/java.jdbc/blob/master/src/test/clojure/clojure/java/test_jdbc.clj#L254) on some databases - I guess those are new keys? 

Alex



--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.
To post to this group, send email to cloju...@googlegroups.com.
Visit this group at http://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/groups/opt_out.

Sean Corfield

unread,
Oct 28, 2013, 5:31:54 PM10/28/13
to cloju...@googlegroups.com
I'll take a look at what's going on. I suspect it's just due to the
vagaries of what comes back in a ResultSet from the Java library and
we (java.jdbc) just convert all the keys to keywords without even
checking that they are valid (as keywords in Clojure).

I'm not sure what the fix should be tho': if you don't control the
input and you expect to be able to call (keyword ..) on it, what
reasonable response can you give to Clojure users of that data?

(Sorry for the slow response - I was away at a cat show in a location
with no 'net access and almost no cell phone service!)

Sean
--
Sean A Corfield -- (904) 302-SEAN
An Architect's View -- http://corfield.org/
World Singles, LLC. -- http://worldsingles.com/

"Perfection is the enemy of the good."
-- Gustave Flaubert, French realist novelist (1821-1880)

Sean Corfield

unread,
Oct 28, 2013, 5:34:03 PM10/28/13
to cloju...@googlegroups.com
Created http://dev.clojure.org/jira/browse/JDBC-71 for tracking this.

Sean Corfield

unread,
Oct 28, 2013, 5:50:18 PM10/28/13
to cloju...@googlegroups.com
I just changed the tests to use (keyword "1") since that works (and I
can't control what the Java library returns and the keyword function
doesn't seem to care what its argument is). java.jdbc now passes its
tests on Clojure master snapshot.

Sean

Sean Corfield

unread,
Oct 28, 2013, 6:07:40 PM10/28/13
to cloju...@googlegroups.com
This broke some World Singles code to. We had :42 in a couple of tests
and a whole map of response codes to messages that had keys like :1-1
:2-4 and so on. Luckily the literals can all be replaced with calls to
(keyword "1-1") etc.

Sean

Alex Miller

unread,
Oct 28, 2013, 8:45:40 PM10/28/13
to cloju...@googlegroups.com
It's totally fine to call keyword - this is about what the reader will read. Comparing those results may just be a bit more awkward.

Alex Miller

unread,
Oct 28, 2013, 8:46:01 PM10/28/13
to cloju...@googlegroups.com
Thanks!

Andy Fingerhut

unread,
Oct 29, 2013, 9:43:58 AM10/29/13
to cloju...@googlegroups.com
It sounds like the current situation with java.jdbc is that it does not require the ability for the reader to read keywords that begin with digits, but it does require that (keyword "1") return a valid keyword :1, at least if there is a column with such a name in a table?

Thanks,
Andy


On Mon, Oct 28, 2013 at 2:31 PM, Sean Corfield <seanco...@gmail.com> wrote:

Sean Corfield

unread,
Oct 29, 2013, 10:21:39 PM10/29/13
to cloju...@googlegroups.com
Yup, since it can't control what comes back in a ResultSet, it expects
to be able to call keyword on any possible column name that might be
valid in some DB somewhere - and also in the somewhat weird ResultSet
objects that can be returned when you do an insert or other mutating
operation. The specific test that was failing was trying to process a
ResultSet object that had two rows, each with a column named "1" and a
data value of NULL... which is clearly not very useful or sane, but
databases spit out bizarre stuff at times :(

Sean

Stuart Halloway

unread,
Oct 31, 2013, 2:06:34 PM10/31/13
to cloju...@googlegroups.com
At Rich's suggestion I have reverted CLJ-1252.

Stu

Sean Corfield

unread,
Oct 31, 2013, 6:42:42 PM10/31/13
to cloju...@googlegroups.com
Could we get a bit more background as to why it has been reverted?
Even tho' it broke a bunch of stuff for us, it still seems like a
reasonable change (and we've already changed our code, and java.jdbc's
tests have been changed too).

Sean

Colin Fleming

unread,
Oct 31, 2013, 10:29:16 PM10/31/13
to cloju...@googlegroups.com
More generally, is there a plan to specify more carefully what is legal for symbols (and keywords etc)? The status quo at the moment is quite painful for tool developers, since the only way to develop tools that accurately parse Clojure if you can't use tools.reader is to read its source code in minute detail and try to reproduce it. Additionally since Clojure is currently very permissive there are currently some pretty crazy symbols in the wild (the other day I had to update my lexer spec after someone reported that the unicode char for '...' was broken as a symbol name even though Clojure accepts it).

http://clojure.org/reader says "Symbols begin with a non-numeric character and can contain alphanumeric characters and *, +, !, -, _, and ? (other characters will be allowed eventually, but not all macro characters have been determined)", however this definition rules out many of Clojure's own symbols (for example < and >). There's lots of other wackiness - the reader page says "Symbols containing / or . are said to be 'qualified'." and yet Clojure currently accepts dotted symbols as namespace aliases.

It's all a little messy right now.

Alex Miller

unread,
Oct 31, 2013, 10:37:18 PM10/31/13
to cloju...@googlegroups.com
Going back to the ticket, the issue was that the reader behavior did not match the specification outlined at http://clojure.org/reader (or the EDN spec that is largely harmonious with it).

However, the change makes the reader more restrictive, thus potentially breaking existing code (such as in java.jdbc). While the breakage we've seen in the contrib libs is relatively minor, it may be more severe in other people's programs. Rich does not take changes that break existing programs lightly and deemed the cure worse than the disease in this case. To his point, no one is really asking that the reader needs to be more restrictive; the issue was that the reader was not harmonious with the spec. An alternate solution is to change the spec or defer the change. In that light, we pulled things back.

I intend to file a ticket regarding the spec and the regex. The cause of the bug was a bad regex that accidentally allows these characters. If we loosen the spec (or the reader's interpretation), we should at least make the regex intentionally match the things we want it to match for clarity.

Make sense?

Alex

Alex Miller

unread,
Oct 31, 2013, 10:55:00 PM10/31/13
to cloju...@googlegroups.com
As per my prior email, I have filed http://dev.clojure.org/jira/browse/CLJ-1286 to address the leading number issue in particular. I think it would be reasonable if touching this part of the spec to try to make it closer to currently reality (as defined by the existing reader behavior). If you want to file at ticket to cover this broader rewrite, that would be helpful to me.

davesann

unread,
Oct 31, 2013, 11:46:34 PM10/31/13
to cloju...@googlegroups.com
Given that: enlive, seesaw and hiccup (at least) use keywords in ways that go well beyond the current spec allowances - and that many people (I suspect) use these - it might be worth considering relaxing some of the supposed, but not generally enforced, constraints further.

Dave

Colin Fleming

unread,
Nov 1, 2013, 4:57:39 PM11/1/13
to cloju...@googlegroups.com
Sure, I will do. I'd be interested to know whether everyone thinks that we're better off restricting the reader, or simply specifying what it does right now. I'm happy with either as long as it's clear what the rules are, and I can imagine that many people would want something more permissive than what http://clojure.org/reader states (it wouldn't allow any symbols in non-English scripts, for example). That page hints at the fact that more macro characters might be needed for Clojure implementation in the future, does anyone have a feeling for how likely this is and what those characters might be?

Sean Corfield

unread,
Nov 1, 2013, 6:28:25 PM11/1/13
to cloju...@googlegroups.com
Given how widespread the use of "strange" keyword literals is at this
point, I think the only realistic choice is to update the spec to
match the reality of what's actually "legal" in practical terms (if
that's possible). My 2¢...

On Fri, Nov 1, 2013 at 1:57 PM, Colin Fleming

Colin Fleming

unread,
Nov 1, 2013, 6:50:23 PM11/1/13
to cloju...@googlegroups.com
That's my feeling too, but if there are concrete or semi-concrete plans to use specific symbols for future Clojure functionality perhaps we should think about excluding those just in case. Since the Clojure ecosystem is still growing fast any pain now will be worse later if we do have to change the spec.
Reply all
Reply to author
Forward
0 new messages