You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "wiedld (via GitHub)" <gi...@apache.org> on 2024/10/15 20:56:53 UTC
[PR] Fix logical vs physical schema mismatch for aliased now(). [datafusion]
wiedld opened a new pull request, #12951:
URL: https://github.com/apache/datafusion/pull/12951
## Which issue does this PR close?
Related to #12733
We found another bug for aliased now() columns.
## Rationale for this change
See the reproducer in the [first commit](https://github.com/apache/datafusion/commit/47f2a927b2ae3850ed8c0c00a3e967762593f27a).
Both of the next two commits are required for the fix.
## What changes are included in this PR?
`get_field_metadata` should handle aliased columns.
Also, the `now()` udf is not nullable.
## Are these changes tested?
Yes.
## Are there any user-facing changes?
Not really.
Only that the now() udf will return not-nullable (instead of the default impl).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix logical vs physical schema mismatch for aliased `now()` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12951:
URL: https://github.com/apache/datafusion/pull/12951#discussion_r1802068436
##########
datafusion/sqllogictest/test_files/metadata.slt:
##########
@@ -125,5 +125,21 @@ NULL NULL l_bar
+query P rowsort
Review Comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix logical vs physical schema mismatch for aliased `now()` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #12951:
URL: https://github.com/apache/datafusion/pull/12951
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix logical vs physical schema mismatch for aliased `now()` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12951:
URL: https://github.com/apache/datafusion/pull/12951#discussion_r1802068212
##########
datafusion/functions/src/datetime/now.rs:
##########
@@ -84,4 +84,8 @@ impl ScalarUDFImpl for NowFunc {
ScalarValue::TimestampNanosecond(now_ts, Some("+00:00".into())),
)))
}
+
+ fn is_nullable(&self, _args: &[Expr], _schema: &dyn ExprSchema) -> bool {
Review Comment:
now can never be nullable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix logical vs physical schema mismatch for aliased `now()` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #12951:
URL: https://github.com/apache/datafusion/pull/12951#discussion_r1803227352
##########
datafusion/sqllogictest/test_files/metadata.slt:
##########
@@ -125,5 +125,21 @@ NULL NULL l_bar
+query P rowsort
+SELECT ts
+FROM ((
+ SELECT now() AS ts
+ FROM table_with_metadata
+) UNION ALL (
+ SELECT ts
+ FROM table_with_metadata
+))
+GROUP BY ts
+ORDER BY ts
+LIMIT 1;
Review Comment:
I verified that without the code in this PR, this test fails like this:
```
External error: query failed: DataFusion error: Schema error: No field named ts. Valid fields are table_with_metadata.id, table_with_metadata.name, table_with_metadata.l_name.
[SQL] SELECT ts
FROM ((
SELECT now() AS ts
FROM table_with_metadata
) UNION ALL (
SELECT ts
FROM table_with_metadata
))
GROUP BY ts
ORDER BY ts
LIMIT 1;
at test_files/metadata.slt:128
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org