You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I've been looking into handling the case where QueryScope::getType receives an instance of SqlFtw\Sql\Expression\Operator as input. Currently, this falls back to simply returning MixedType, which renders the AST-derived types mostly useless. It would be much more powerful if we could get the true type by applying the operator.
Most of the operators will have a direct analog to PHP operators, and so the logic in PHPStan\Analyser\MutatingScope and PHPStan\Reflection\InitializerExprTypeResolver for handling operators has much of the logic we need already. However, this logic handles types that are irrelevant to SQL as well (such as arrays etc.), and it is buried so deep in the PHPStan architecture that it is likely unreasonable (or inappropriate) to use within QueryScope.
If we want to support SQL operators, would we need to duplicate all this logic from PHPStan? Even if we omit the parts that are irrelevant to SQL, this seems like it would be quite a lot of code duplication. Would it be reasonable to consider refactoring PHPStan to create a generic set of "type math" classes?
@staabm I'd be very grateful to hear your opinions about how to approach this, and whether or not it's something you'd like me to work on. Thanks!
The text was updated successfully, but these errors were encountered:
If we want to support SQL operators, would we need to duplicate all this logic from PHPStan? Even if we omit the parts that are irrelevant to SQL, this seems like it would be quite a lot of code duplication. Would it be reasonable to consider refactoring PHPStan to create a generic set of "type math" classes?
I think we could take different approaches. I am not sure the logic required is really 100% identical, therefore I guess it would make sense to build it in phpstan-dba itself.
I think we can/should directly base it on the SQL AST Nodes. the phpstan logic is based on nikic/php-parser AST nodes. we could transform the SQL AST Nodes into equivalent nikic/php-parser nodes and pass it into phpstan, but then I think the logic in phpstan would be to php-specific and in the end will not reflect the sql context/semantics we need.
summary: I think we should implement it our own for now and do it in a separate math class based on sql ast nodes
whether there is room for re-use can be decided after we implemented a rough prototype and we see similarities or not
I've been looking into handling the case where
QueryScope::getType
receives an instance ofSqlFtw\Sql\Expression\Operator
as input. Currently, this falls back to simply returningMixedType
, which renders the AST-derived types mostly useless. It would be much more powerful if we could get the true type by applying the operator.Most of the operators will have a direct analog to PHP operators, and so the logic in
PHPStan\Analyser\MutatingScope
andPHPStan\Reflection\InitializerExprTypeResolver
for handling operators has much of the logic we need already. However, this logic handles types that are irrelevant to SQL as well (such as arrays etc.), and it is buried so deep in the PHPStan architecture that it is likely unreasonable (or inappropriate) to use withinQueryScope
.If we want to support SQL operators, would we need to duplicate all this logic from PHPStan? Even if we omit the parts that are irrelevant to SQL, this seems like it would be quite a lot of code duplication. Would it be reasonable to consider refactoring PHPStan to create a generic set of "type math" classes?
@staabm I'd be very grateful to hear your opinions about how to approach this, and whether or not it's something you'd like me to work on. Thanks!
The text was updated successfully, but these errors were encountered: