Some small tweaks to Red. #404
Conversation
|
It seems 2 tests are failing with these changes... |
lib/Red/Driver/Pg.pm6
Outdated
| @@ -34,6 +34,22 @@ multi method translate(Red::AST::Select $_, $context?, :$gambi where !*.defined) | |||
| self.Red::Driver::CommonSQL::translate($_, $context, :gambi); | |||
| } | |||
|
|
|||
| multi method translate(Red::AST::In $_, $context?) { | |||
| if .right.value ~~ Positional { | |||
There was a problem hiding this comment.
Could you, please, put this test on the signature and remove the else?
There was a problem hiding this comment.
I mean something like:
multi method translate(Red::AST::In $_ where .right.value ~~ Positional, $context?) {There was a problem hiding this comment.
But, instead of testing the right's value type, it would be better to test if it's an Red::AST::Value and if its .type is Positional.
multi method translate(Red::AST::In $_ where .right ~~ Red::AST::Value && .right.type ~~ Positional, $context?) {
lib/Red/Driver/Pg.pm6
Outdated
| my ($lstr, @lbind) := do given self.translate: .left, $context { .key, .value } | ||
|
|
||
| if .right.value.elems == 0 { | ||
| return "$lstr { .op } (SELECT 0 WHERE false)" => @lbind; |
There was a problem hiding this comment.
Why change to use in (SELECT 0 WHERE false)? why not in ()?
There was a problem hiding this comment.
Because In Pg, the in () will raise error, So, we should add placehoder statement to select "nothing".
There was a problem hiding this comment.
In that way, maybe we could do an:
multi method translate(Red::AST::In $_ where .right ~~ Red::AST::Value && .right.type ~~ Positional && .right.elems, $context?) {?
lib/Red/Driver/Pg.pm6
Outdated
| @@ -34,6 +34,22 @@ multi method translate(Red::AST::Select $_, $context?, :$gambi where !*.defined) | |||
| self.Red::Driver::CommonSQL::translate($_, $context, :gambi); | |||
| } | |||
|
|
|||
| multi method translate(Red::AST::In $_, $context?) { | |||
There was a problem hiding this comment.
Isn't it common? Couldn't it be on CommonSQL?
There was a problem hiding this comment.
It's common, But It's strange that it works in SQLite.
lib/Red/Driver/Pg.pm6
Outdated
| return "$lstr { .op } (SELECT 0 WHERE false)" => @lbind; | ||
| } | ||
|
|
||
| my $in-placeholder = '(' ~ (self.wildcard xx .right.value.elems).join(',') ~ ')'; |
There was a problem hiding this comment.
If you fix one of my errors (https://github.com/FCO/Red/blob/master/lib/Red/Driver/CommonSQL.pm6#L521) changing "?" to self.wildcard, you could just use the self.translate: .right, $context
| @@ -111,6 +127,10 @@ multi method default-type-for(Red::Column $ where .attr.type ~~ Bool | |||
| multi method default-type-for(Red::Column $ where .attr.type ~~ UUID --> Str:D) {"uuid"} | |||
| multi method default-type-for(Red::Column $ --> Str:D) {"varchar(255)"} | |||
|
|
|||
| multi method type-by-name("text" --> "text") {} | |||
| multi method type-by-name("json" --> "json") {} | |||
There was a problem hiding this comment.
Could you, please, add a test for json and jsonb?
for SQLite, it re-fetches the row from the model by using LastInsertedRow
class, Then the `.row{@ids}` does the right thing all the time. But in
postgres, the $data will filled by `INSERT INTO table xxx RETURNING *` after
the RETURNING, we'll $data with column names with underscores.
So, in the pg case, We convert the $data structure according to the model, make
$data look like the same as fetched with LastInsertedRow class.
|
|
||
| model Category is table<test_category> { | ||
| has Int $.id is serial; | ||
| has Int $.parent_id is column{ :references{ Category.id }, :nullable, }; |
There was a problem hiding this comment.
This test is failing here. For Red:api<2> you need to set the :model<> for is referencing or :model-name<> in this case... and receive the model type as parameter:
has Int $.parent_if is column{ :references{ .id }, :nullable };
Hi, These 3 patches add method column-values, and some small fixes for Red::Driver::Pg
Thanks!