Narrow types via join / join-condition #531
Conversation
|
@gharlan could you do me a favour and verify the expected types? |
| assertType('PDOStatement<array{adaid: int<-32768, 32767>, 0: int<-32768, 32767>, akid: int<-2147483648, 2147483647>, 1: int<-2147483648, 2147483647>}>', $stmt); | ||
|
|
||
| $stmt = $pdo->query('SELECT adaid, akid from ada left join ak on (adaid = eladaid)'); | ||
| assertType('PDOStatement<array{adaid: int<-32768, 32767>, 0: int<-32768, 32767>, akid: int<-2147483648, 2147483647>|null, 1: int<-2147483648, 2147483647>|null}>', $stmt); |
There was a problem hiding this comment.
rows not matched in the main result, will be filled up with null, therefore even a non-nullable column can get null on outer joins
| $stmt = $pdo->query('SELECT adaid, eladaid from ada join ak on (adaid = eladaid)'); | ||
| assertType('PDOStatement<array{adaid: int<-32768, 32767>, 0: int<-32768, 32767>, eladaid: int<-2147483648, 2147483647>, 1: int<-2147483648, 2147483647>}>', $stmt); | ||
|
|
||
| $stmt = $pdo->query('SELECT adaid, eladaid from ada inner join ak on (adaid = eladaid)'); |
There was a problem hiding this comment.
on inner join, nullable columns get non nullable
staabm
force-pushed
the
joins
branch
2 times, most recently
from
b15c06c to
2cab3d4
Compare
| /** | ||
| * @api | ||
| */ | ||
| final class Join |
There was a problem hiding this comment.
not sure it makes sense to model joins here. maybe we should instead use SqlFtw\Sql\Dml\TableReference\Join instead, as we need the low level information
src/SqlAst/QueryScope.php
Outdated
| if ($join->getType() === Join::TYPE_OUTER) { | ||
| return TypeCombinator::addNull($column->getType()); | ||
| } | ||
| return TypeCombinator::removeNull($column->getType()); |
There was a problem hiding this comment.
I think we should only add/remove NULLs for columns which are part of the join condition, but not regular columns selected from joined tables - without beeing part of the join condition.
There was a problem hiding this comment.
before doing so, we may wait a bit for sqlftw stabilization as we would need a deep dive into the AST
staabm
changed the title
closes #527