sql: allow CREATE UNLOGGED TABLE syntax #52596
Conversation
|
This change is |
otan
force-pushed
the
unlogged_table
branch
4 times, most recently
from
18e7bf4
to
d57c5c1
Compare
|
If it does nothing, why are you storing it on the table descriptor? |
|
I don't know where to stop! I may have to implement SET UNLOGGED and SET LOGGED and it needs that to be set to behave correctly, hehe. |
|
I don't think we should be writing this information to disk if it is to be used just to play with these commands. Why can't we just parse and noop the related commands? |
|
i honestly could be swayed either way. i don't want a no-op and "parse through" in the sql.y layer as i do want an explicit warning/notice, which you cannot do from the sql.y bit. |
|
I agree that you'll have to implement some plan nodes, but I'd prefer for those to be noops than having to do something to the table |
|
alright well, change made :P |
pkg/sql/show_create.go
Outdated
| @@ -73,6 +73,9 @@ func ShowCreateTable( | |||
|
|
|||
| f := tree.NewFmtCtx(tree.FmtSimple) | |||
| f.WriteString("CREATE ") | |||
| if desc.Unlogged { | |||
| f.WriteString("UNLOGGED ") | |||
| } | |||
There was a problem hiding this comment.
I agree with @rohany. If the UNLOGGED keyword does nothing, I think it's misleading to label the table as UNLOGGED in SHOW CREATE. If you remove this, I think you can avoid storing anything new in a descriptor, no?
There was a problem hiding this comment.
thought i removed this on cleanup!
In SQL, a CREATE TABLE can take in TEMP | UNLOGGED. Accordingly, I've updated the sql.y to account for this, changing the opt_temp from a bool to a `tree.Persistence` ENUM that can be PERMANENT, TEMPORARY or UNLOGGED. This introduced some downstream changes to various package to recognize these arguments. Note sequences and views cannot be unlogged. This makes up the majority of the changes. Otherwise, recognize UNLOGGED TABLE in teh parser. Add a warning that the use of UNLOGGED TABLE does not actually do anything. Note this is always safe as UNLOGGED TABLE is a performance improvement in PostgreSQL by sacrificing crash-safety. In our case, we'll still always have safety features and thus still be correct, albeit slower. Release note (sql change): Allow `CREATE UNLOGGED TABLE` syntax to be parsed and recorded internally within CockroachDB. However, this syntax does not modify behavior of the table.
|
thankssss bors r=rohany |
|
bors bors the magical fruit bors r=rohany |
|
bors? flaky? unheard of. |
|
Build succeeded: |
In SQL, a CREATE TABLE can take in TEMP | UNLOGGED. Accordingly, I've
updated the sql.y to account for this, changing the opt_temp from a bool
to a
tree.PersistenceENUM that can be PERMANENT, TEMPORARY orUNLOGGED. This introduced some downstream changes to various package to
recognize these arguments. Note sequences and views cannot be unlogged.
This makes up the majority of the changes.
Otherwise, recognize UNLOGGED TABLE in teh parser. Add a warning that the
use of UNLOGGED TABLE does not actually do anything.
Note this is always safe as UNLOGGED TABLE is a performance improvement
in PostgreSQL by sacrificing crash-safety. In our case, we'll still always
have safety features and thus still be correct, albeit slower.
Release note (sql change): Allow
CREATE UNLOGGED TABLEsyntax to beparsed and recorded internally within CockroachDB. However, this syntax
does not modify behavior of the table.
Resolves #43352.
Refs #51818