JakeWheat
Sqlite: `CURRENT_TIMESTAMP`
Consider the following column def:
created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP,
This will not parse (even if CURRENT_TIMESTAMP is marked as a keyword). I have tried looking through the parser code, but couldn't find the exact spot I'd have to make a change. Would I need to introduce an additional "constant" array on the dialect, and add an extra branch to term which parses any one of those constants? Or is something like this already covered by another one of the branches?
CURRENT_TIMESTAMP should not be a keyword for this to parse. It's a different issue:
These both parse with the default dialect:
created_at DATETIME default CURRENT_TIMESTAMP
created_at DATETIME DEFAULT CURRENT_TIMESTAMP NOT NULL
The second one isn't how it's usually written, but I think the SQL standard specifies this order: http://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#_11_4_column_definition . Let me know if you agree, in case I'm missing something.
So to support this, you want a different dialect option which allows the default clause to be mixed in with the column constraints, not sure what to call the option. (Both postgres and sqlite grammars lump default in with column constraints, I assume most other systems also do this, but, calling a default clause a column constraint makes no sense to me.)
Hi @JakeWheat . I am working on a PR for this. It looks like sqlite allows more than one DEFAULT clause to exist (I've tried it in the repl). The last clause is the one that gets used. I assume this lib should stay as close to the ast as possible, so I guess I should change the Maybe DefaultClause to a list instead (behind a dialect flag, of course)
It also looks like sqlite defines DEFAULT clauses as constraints (see this diagram), so I think for generality it should be included in the same type with the other constraints (otherwise we wouldn't be able to represent the actual ast) (even if it's only allowed in the position it's allowed right now unless the user doesn't enable the dialect flag)
I think for generality it should be included in the same type with the other constraints
Yes, I agree this is a sensible way to support it, and the parser should support this as a dialect option since it's the normal way to write SQL.
I guess I should change the Maybe DefaultClause to a list instead
This is a problem supporting SQL, you end up with a very general AST which is less pleasant to use. Is there some common SQL that you see that has multiple default clauses like this I could take a look at? But, if the AST represents ConstraintOrDefault as a list, which is the obvious way to do it, then this AST will support multiple defaults more straightforwardly than not supporting them.
I'll have a think about the syntax types for columns without types, I assume this is common in sqlite, but it's a particularly big impostion on users of the code. Let me know if you have any alternative suggestions.
Sorry for the delay, I'll try to check and merge the pull request soon.
I'll have a think about the syntax types for columns without types
I thought about it and decided I was wrong, that given everything we have to deal with on a syntax that supports a range of different SQL dialects, turning the type in column definitions to a Maybe is not a big deal.