You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "benrsatori (via GitHub)" <gi...@apache.org> on 2025/02/11 08:54:55 UTC
[PR] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
benrsatori opened a new pull request, #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723
Add support for PostgreSQL and Redshift geometric operators.
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1959289112
##########
src/ast/data_type.rs:
##########
@@ -386,6 +386,8 @@ pub enum DataType {
///
/// [bigquery]: https://cloud.google.com/bigquery/docs/user-defined-functions#templated-sql-udf-parameters
AnyType,
+ /// geometric type
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953366365
##########
tests/sqlparser_postgres.rs:
##########
@@ -2057,13 +2057,13 @@ fn parse_pg_custom_binary_ops() {
// Here, we test the ones used by common extensions
let operators = [
// PostGIS
- "&&&", // n-D bounding boxes intersect
- "&<", // (is strictly to the left of)
- "&>", // (is strictly to the right of)
+ "&&&", // n-D bounding boxes intersect
+ //"&<", // (is strictly to the left of) // Same as OverlapsToLeft, causes duplicate failure
+ //"&>", // (is strictly to the right of) // Same as OverlapsToRight, causes duplicate failure
"|=|", // distance between A and B trajectories at their closest point of approach
"<<#>>", // n-D distance between A and B bounding boxes
- "|>>", // A's bounding box is strictly above B's.
- "~=", // bounding box is the same
+ //"|>>", // A's bounding box is strictly above B's. // Same as IsStrictlyAbove, causes duplicate failure
+ //"~=", // bounding box is the same // Same as SameAs, causes duplicate failure
Review Comment:
The commented-out operators are already defined under different names in the code,
so they can no longer be considered custom operators.
If we include these again under their original symbols, it causes a duplicate match failure.
so I have now completely removed them.
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953348166
##########
src/parser/mod.rs:
##########
@@ -1350,7 +1351,15 @@ impl<'a> Parser<'a> {
Ok(Some(expr)) => Ok(expr),
// No expression prefix associated with this word
- Ok(None) => Ok(self.parse_expr_prefix_by_unreserved_word(&w, span)?),
+ Ok(None) => {
+ if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect)
+ && GEOMETRIC_TYPES.contains(&w.keyword)
+ {
+ self.parse_geometric_type(w.keyword)
Review Comment:
done
##########
src/parser/mod.rs:
##########
@@ -1496,6 +1532,33 @@ impl<'a> Parser<'a> {
}
}
+ fn parse_geometric_type(&mut self, keyword: Keyword) -> Result<Expr, ParserError> {
+ let token_with_span = self.next_token(); // Get the full TokenWithSpan
+ let value = match token_with_span.token {
+ // Extract the Token field
+ Token::SingleQuotedString(s) => Value::SingleQuotedString(s),
+ _ => return self.expected("SingleQuotedString", token_with_span), // Pass full TokenWithSpan
+ };
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1956397660
##########
src/tokenizer.rs:
##########
@@ -1308,6 +1356,28 @@ impl<'a> Tokenizer<'a> {
_ => self.start_binop(chars, "||", Token::StringConcat),
}
}
+ Some('&') if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect) => {
Review Comment:
similarly for these clauses it would be nice to use the `self.dialect.supports_geometric_types()` method
##########
src/ast/data_type.rs:
##########
@@ -879,3 +882,29 @@ pub enum ArrayElemTypeDef {
/// `Array(Int64)`
Parenthesis(Box<DataType>),
}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
Review Comment:
Could we add a short description to this struct with a link to the docs?
##########
src/parser/mod.rs:
##########
@@ -3079,6 +3128,55 @@ impl<'a> Parser<'a> {
Token::QuestionAnd => Some(BinaryOperator::QuestionAnd),
Token::QuestionPipe => Some(BinaryOperator::QuestionPipe),
Token::CustomBinaryOperator(s) => Some(BinaryOperator::Custom(s.clone())),
+ Token::DoubleSharp if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect) => {
Review Comment:
for these clauses as well could we use the introduced `self.dialect.supports_geometric_types()`?
##########
src/dialect/mod.rs:
##########
@@ -912,6 +927,10 @@ pub trait Dialect: Debug + Any {
fn supports_array_typedef_size(&self) -> bool {
false
}
+ /// Returns true if the dialect supports geometric types.
Review Comment:
Could we add a link to the postgres docs here (and maybe an example sql)? it would help folks with context on what we mean by geometric types
##########
src/ast/operator.rs:
##########
@@ -53,6 +53,21 @@ pub enum UnaryOperator {
PGAbs,
/// Unary logical not operator: e.g. `! false` (Hive-specific)
BangNot,
+ /// `#` Number of points in path or polygon (PostgreSQL/Redshift geometric operator)
+ /// see <https://www.postgresql.org/docs/9.5/functions-geometry.html>
+ NumOfPoints,
Review Comment:
Can we do similar to the binary operator where we name them according to their symbols to be dialect agnostic?
##########
src/ast/data_type.rs:
##########
@@ -386,6 +386,8 @@ pub enum DataType {
///
/// [bigquery]: https://cloud.google.com/bigquery/docs/user-defined-functions#templated-sql-udf-parameters
AnyType,
+ /// geometric type
Review Comment:
Could we include a link to the postgres docs here?
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953347665
##########
src/ast/operator.rs:
##########
@@ -253,6 +268,40 @@ pub enum BinaryOperator {
/// Specifies a test for an overlap between two datetime periods:
/// <https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#overlaps-predicate>
Overlaps,
+ /// `##` Point of closest proximity (PostgreSQL/Redshift geometric operator), e.g. `point '(0,0)' ## lseg '((2,0),(0,2))'`
+ PointOfClosestProximity,
+ /// `<->` Distance between (PostgreSQL/Redshift geometric operator), e.g. `circle '((0,0),1)' <-> circle '((5,0),1)'`
+ DistanceBetween,
+ /// `&&` Overlaps? (PostgreSQL/Redshift geometric operator), e.g. box '((0,0),(1,1))' && box '((0,0),(2,2))'
Review Comment:
no, there was one commented out, I removed it.
##########
src/keywords.rs:
##########
@@ -141,6 +141,7 @@ define_keywords!(
BOOL,
BOOLEAN,
BOTH,
+ BOX, // PostgreSQL/Redshift geometry type, rectangular box
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1959357480
##########
src/ast/operator.rs:
##########
@@ -53,6 +53,21 @@ pub enum UnaryOperator {
PGAbs,
/// Unary logical not operator: e.g. `! false` (Hive-specific)
BangNot,
+ /// `#` Number of points in path or polygon (PostgreSQL/Redshift geometric operator)
+ /// see <https://www.postgresql.org/docs/9.5/functions-geometry.html>
+ NumOfPoints,
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953345692
##########
src/ast/operator.rs:
##########
@@ -253,6 +268,40 @@ pub enum BinaryOperator {
/// Specifies a test for an overlap between two datetime periods:
/// <https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#overlaps-predicate>
Overlaps,
+ /// `##` Point of closest proximity (PostgreSQL/Redshift geometric operator), e.g. `point '(0,0)' ## lseg '((2,0),(0,2))'`
+ PointOfClosestProximity,
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#issuecomment-2671531452
Hi @iffyio
I fixed the CI
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953345106
##########
src/ast/operator.rs:
##########
@@ -53,6 +53,16 @@ pub enum UnaryOperator {
PGAbs,
/// Unary logical not operator: e.g. `! false` (Hive-specific)
BangNot,
+ /// `#` Number of points in path or polygon (PostgreSQL/Redshift geometric operator), e.g. `# path '((1,0),(0,1),(-1,0))'`
+ NumOfPoints,
+ /// `@-@` Length or circumference (PostgreSQL/Redshift geometric operator), e.g. `@-@ path '((0,0),(1,0))'`
+ LenOfCircumference,
Review Comment:
it's a typo, fixed
##########
src/ast/operator.rs:
##########
@@ -53,6 +53,16 @@ pub enum UnaryOperator {
PGAbs,
/// Unary logical not operator: e.g. `! false` (Hive-specific)
BangNot,
+ /// `#` Number of points in path or polygon (PostgreSQL/Redshift geometric operator), e.g. `# path '((1,0),(0,1),(-1,0))'`
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1959336151
##########
src/tokenizer.rs:
##########
@@ -1308,6 +1356,28 @@ impl<'a> Tokenizer<'a> {
_ => self.start_binop(chars, "||", Token::StringConcat),
}
}
+ Some('&') if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect) => {
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1951078414
##########
src/ast/operator.rs:
##########
@@ -53,6 +53,16 @@ pub enum UnaryOperator {
PGAbs,
/// Unary logical not operator: e.g. `! false` (Hive-specific)
BangNot,
+ /// `#` Number of points in path or polygon (PostgreSQL/Redshift geometric operator), e.g. `# path '((1,0),(0,1),(-1,0))'`
Review Comment:
Could we include a link to the docs where the operators are described? that would be helpful for folks that come across it and need to find more context
##########
src/ast/operator.rs:
##########
@@ -77,13 +92,13 @@ impl fmt::Display for UnaryOperator {
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
pub enum BinaryOperator {
- /// Plus, e.g. `a + b`
+ /// `+` Addition, also Translation (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(1,1))' + point '(2.0,0)'`
Plus,
- /// Minus, e.g. `a - b`
+ /// `-` Subtraction, Translation (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(1,1))' - point '(2.0,0)'`
Minus,
- /// Multiply, e.g. `a * b`
+ /// `*` Multiplication, also Scaling (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(1,1))' * point '(2.0,0)'`
Multiply,
- /// Divide, e.g. `a / b`
+ /// `/` Division, also Scaling (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(2,2))' / point '(2.0,0)'`
Review Comment:
Can we skip this diff? I'm thinking since these operators are fundamental and apply to various scenarios, no need to mention specific dialect semantics given neither the parser nor AST handles them specially as a result
##########
src/keywords.rs:
##########
@@ -141,6 +141,7 @@ define_keywords!(
BOOL,
BOOLEAN,
BOTH,
+ BOX, // PostgreSQL/Redshift geometry type, rectangular box
Review Comment:
same lets drop these comments, the keywords will likely be applicable in other contexts
##########
src/parser/mod.rs:
##########
@@ -1350,7 +1351,15 @@ impl<'a> Parser<'a> {
Ok(Some(expr)) => Ok(expr),
// No expression prefix associated with this word
- Ok(None) => Ok(self.parse_expr_prefix_by_unreserved_word(&w, span)?),
+ Ok(None) => {
+ if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect)
+ && GEOMETRIC_TYPES.contains(&w.keyword)
+ {
+ self.parse_geometric_type(w.keyword)
Review Comment:
could this be its own branch within `parse_expr_prefix_by_reserved_word` instead?
It looks like we should be able to do something like
```rust
kw if self.dialect.supports_pg_geometric_types() && PG_GEOMETRIC_TYPES.contains(kw) => {
self.parse_geometric_expr(kw)
}
```
##########
tests/sqlparser_postgres.rs:
##########
@@ -2057,13 +2057,13 @@ fn parse_pg_custom_binary_ops() {
// Here, we test the ones used by common extensions
let operators = [
// PostGIS
- "&&&", // n-D bounding boxes intersect
- "&<", // (is strictly to the left of)
- "&>", // (is strictly to the right of)
+ "&&&", // n-D bounding boxes intersect
+ //"&<", // (is strictly to the left of) // Same as OverlapsToLeft, causes duplicate failure
+ //"&>", // (is strictly to the right of) // Same as OverlapsToRight, causes duplicate failure
"|=|", // distance between A and B trajectories at their closest point of approach
"<<#>>", // n-D distance between A and B bounding boxes
- "|>>", // A's bounding box is strictly above B's.
- "~=", // bounding box is the same
+ //"|>>", // A's bounding box is strictly above B's. // Same as IsStrictlyAbove, causes duplicate failure
+ //"~=", // bounding box is the same // Same as SameAs, causes duplicate failure
Review Comment:
not sure I followed why these are commented out, what do we mean by causes duplicate failure?
##########
src/ast/operator.rs:
##########
@@ -253,6 +268,40 @@ pub enum BinaryOperator {
/// Specifies a test for an overlap between two datetime periods:
/// <https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#overlaps-predicate>
Overlaps,
+ /// `##` Point of closest proximity (PostgreSQL/Redshift geometric operator), e.g. `point '(0,0)' ## lseg '((2,0),(0,2))'`
+ PointOfClosestProximity,
Review Comment:
hmm actually thinking about it, I feel like the naming of the operators might become problematic, e.g. if another dialect supports `##` operator then calling it `PointOfClosestProximity` would be confusing vs if the operator was called `DoubleHash` dialect agnostic. Would it make sense to name the operators in such a manner? i.e. by their characters or if theres a well known description that isnt describing the pg behavior
##########
src/ast/operator.rs:
##########
@@ -53,6 +53,16 @@ pub enum UnaryOperator {
PGAbs,
/// Unary logical not operator: e.g. `! false` (Hive-specific)
BangNot,
+ /// `#` Number of points in path or polygon (PostgreSQL/Redshift geometric operator), e.g. `# path '((1,0),(0,1),(-1,0))'`
+ NumOfPoints,
+ /// `@-@` Length or circumference (PostgreSQL/Redshift geometric operator), e.g. `@-@ path '((0,0),(1,0))'`
+ LenOfCircumference,
Review Comment:
the doc says length _or_ circumference, is the `Of` a typo?
##########
src/parser/mod.rs:
##########
@@ -1411,6 +1420,33 @@ impl<'a> Parser<'a> {
),
})
}
+ tok @ Token::Sharp
+ | tok @ Token::AtDashAt
+ | tok @ Token::AtAt
+ | tok @ Token::QuestionMarkDash
+ | tok @ Token::QuestionPipe
+ if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect) =>
Review Comment:
Can we replace these with a dialect method? the `dialect_of` has been deprecated, we can introduce a method like `supports_pg_geometric_types()` or similar
##########
src/test_utils.rs:
##########
@@ -284,6 +284,17 @@ where
dialects
}
+pub fn only_psql_redshift() -> TestedDialects {
+ TestedDialects {
+ dialects: vec![
+ Box::new(PostgreSqlDialect {}),
+ Box::new(RedshiftSqlDialect {}),
+ ],
+ options: None,
+ recursion_limit: None,
+ }
+}
+
Review Comment:
by switching to the dialect method, it would be possible to reuse `all_dialects_where` in place of this
##########
src/parser/mod.rs:
##########
@@ -1496,6 +1532,33 @@ impl<'a> Parser<'a> {
}
}
+ fn parse_geometric_type(&mut self, keyword: Keyword) -> Result<Expr, ParserError> {
+ let token_with_span = self.next_token(); // Get the full TokenWithSpan
+ let value = match token_with_span.token {
+ // Extract the Token field
+ Token::SingleQuotedString(s) => Value::SingleQuotedString(s),
+ _ => return self.expected("SingleQuotedString", token_with_span), // Pass full TokenWithSpan
+ };
Review Comment:
```suggestion
let token_with_span = self.next_token();
let value = match token_with_span.token {
Token::SingleQuotedString(s) => Value::SingleQuotedString(s),
_ => return self.expected("SingleQuotedString", token_with_span),
};
```
thinking we can drop the comments in this case since the code is as descriptive
##########
src/ast/operator.rs:
##########
@@ -253,6 +268,40 @@ pub enum BinaryOperator {
/// Specifies a test for an overlap between two datetime periods:
/// <https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#overlaps-predicate>
Overlaps,
+ /// `##` Point of closest proximity (PostgreSQL/Redshift geometric operator), e.g. `point '(0,0)' ## lseg '((2,0),(0,2))'`
+ PointOfClosestProximity,
+ /// `<->` Distance between (PostgreSQL/Redshift geometric operator), e.g. `circle '((0,0),1)' <-> circle '((5,0),1)'`
+ DistanceBetween,
+ /// `&&` Overlaps? (PostgreSQL/Redshift geometric operator), e.g. box '((0,0),(1,1))' && box '((0,0),(2,2))'
Review Comment:
are we missing a variant for this doc comment?
##########
src/ast/operator.rs:
##########
@@ -121,11 +136,11 @@ pub enum BinaryOperator {
MyIntegerDivide,
/// Support for custom operators (such as Postgres custom operators)
Custom(String),
- /// Bitwise XOR, e.g. `a # b` (PostgreSQL-specific)
+ // `#` PostgreSQL Bitwise XOR, also Intersection (PostgreSQL/Redshift geometric operator), e.g. `box '((1,-1),(-1,1))' # box '((1,1),(-2,-2))'`
PGBitwiseXor,
- /// Bitwise shift left, e.g. `a << b` (PostgreSQL-specific)
+ /// `<<` PostgreSQL Bitwise Shift Left, also Left of? (PostgreSQL/Redshift geometric operator), e.g. circle '((0,0),1)' << circle '((5,0),1)'
PGBitwiseShiftLeft,
- /// Bitwise shift right, e.g. `a >> b` (PostgreSQL-specific)
+ /// `>>` PostgreSQL Bitwise Shift Right, Right of? (PostgreSQL/Redshift geometric operator), e.g. circle '((5,0),1)' >> circle '((0,0),1)'
Review Comment:
actually same with these, I think just mentioning the operator suffices, without going into semantics
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1959458718
##########
src/test_utils.rs:
##########
@@ -284,6 +284,17 @@ where
dialects
}
+pub fn only_psql_redshift() -> TestedDialects {
+ TestedDialects {
+ dialects: vec![
+ Box::new(PostgreSqlDialect {}),
+ Box::new(RedshiftSqlDialect {}),
+ ],
+ options: None,
+ recursion_limit: None,
+ }
+}
+
Review Comment:
Updated the code to use `all_dialects_where` as to your suggestion
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953345423
##########
src/ast/operator.rs:
##########
@@ -121,11 +136,11 @@ pub enum BinaryOperator {
MyIntegerDivide,
/// Support for custom operators (such as Postgres custom operators)
Custom(String),
- /// Bitwise XOR, e.g. `a # b` (PostgreSQL-specific)
+ // `#` PostgreSQL Bitwise XOR, also Intersection (PostgreSQL/Redshift geometric operator), e.g. `box '((1,-1),(-1,1))' # box '((1,1),(-2,-2))'`
PGBitwiseXor,
- /// Bitwise shift left, e.g. `a << b` (PostgreSQL-specific)
+ /// `<<` PostgreSQL Bitwise Shift Left, also Left of? (PostgreSQL/Redshift geometric operator), e.g. circle '((0,0),1)' << circle '((5,0),1)'
PGBitwiseShiftLeft,
- /// Bitwise shift right, e.g. `a >> b` (PostgreSQL-specific)
+ /// `>>` PostgreSQL Bitwise Shift Right, Right of? (PostgreSQL/Redshift geometric operator), e.g. circle '((5,0),1)' >> circle '((0,0),1)'
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953349274
##########
src/test_utils.rs:
##########
@@ -284,6 +284,17 @@ where
dialects
}
+pub fn only_psql_redshift() -> TestedDialects {
+ TestedDialects {
+ dialects: vec![
+ Box::new(PostgreSqlDialect {}),
+ Box::new(RedshiftSqlDialect {}),
+ ],
+ options: None,
+ recursion_limit: None,
+ }
+}
+
Review Comment:
I removed it
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#issuecomment-2676808148
🚀
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1959286405
##########
src/dialect/mod.rs:
##########
@@ -912,6 +927,10 @@ pub trait Dialect: Debug + Any {
fn supports_array_typedef_size(&self) -> bool {
false
}
+ /// Returns true if the dialect supports geometric types.
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953344303
##########
src/ast/operator.rs:
##########
@@ -77,13 +92,13 @@ impl fmt::Display for UnaryOperator {
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
pub enum BinaryOperator {
- /// Plus, e.g. `a + b`
+ /// `+` Addition, also Translation (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(1,1))' + point '(2.0,0)'`
Plus,
- /// Minus, e.g. `a - b`
+ /// `-` Subtraction, Translation (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(1,1))' - point '(2.0,0)'`
Minus,
- /// Multiply, e.g. `a * b`
+ /// `*` Multiplication, also Scaling (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(1,1))' * point '(2.0,0)'`
Multiply,
- /// Divide, e.g. `a / b`
+ /// `/` Division, also Scaling (PostgreSQL/Redshift geometric operator), e.g. `box '((0,0),(2,2))' / point '(2.0,0)'`
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1953348633
##########
src/parser/mod.rs:
##########
@@ -1411,6 +1420,33 @@ impl<'a> Parser<'a> {
),
})
}
+ tok @ Token::Sharp
+ | tok @ Token::AtDashAt
+ | tok @ Token::AtAt
+ | tok @ Token::QuestionMarkDash
+ | tok @ Token::QuestionPipe
+ if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect) =>
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1959303970
##########
src/ast/data_type.rs:
##########
@@ -879,3 +882,29 @@ pub enum ArrayElemTypeDef {
/// `Array(Int64)`
Parenthesis(Box<DataType>),
}
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
Review Comment:
done
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#issuecomment-2668754883
@benrsatori could you take a look at the CI failures?
--
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] Add support for PostgreSQL/Redshift geometric operators [datafusion-sqlparser-rs]
Posted by "benrsatori (via GitHub)" <gi...@apache.org>.
benrsatori commented on code in PR #1723:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/1723#discussion_r1959329022
##########
src/parser/mod.rs:
##########
@@ -3079,6 +3128,55 @@ impl<'a> Parser<'a> {
Token::QuestionAnd => Some(BinaryOperator::QuestionAnd),
Token::QuestionPipe => Some(BinaryOperator::QuestionPipe),
Token::CustomBinaryOperator(s) => Some(BinaryOperator::Custom(s.clone())),
+ Token::DoubleSharp if dialect_of!(self is PostgreSqlDialect | RedshiftSqlDialect) => {
Review Comment:
done
--
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