You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "kfirSatori (via GitHub)" <gi...@apache.org> on 2026/06/01 14:03:16 UTC
[PR] Add xmlnamespace support mssql [datafusion-sqlparser-rs]
kfirSatori opened a new pull request, #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361
Added support to parse xmlnamespace in mssql, for queris like:
`WITH XMLNAMESPACES ('urn:test' AS ns)
SELECT 1 AS [ns:Value]
FOR XML PATH('ns:Root');`
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3683526418
##########
src/ast/query.rs:
##########
@@ -756,11 +756,23 @@ pub struct With {
pub recursive: bool,
/// The list of CTEs declared by this `WITH` clause.
pub cte_tables: Vec<Cte>,
+ /// Optional XML namespace definitions (`WITH XMLNAMESPACES (...)`).
+ pub xml_namespaces: Vec<XmlNamespaceDefinition>,
Review Comment:
Indeed i use the WITH keyword as done with cte_tables, i hope i understood what you have meant.
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3674840779
##########
tests/sqlparser_mssql.rs:
##########
@@ -2926,22 +2926,7 @@ fn parse_mssql_money_constants() {
#[test]
fn parse_xmlnamespaces() {
- let sql = r#"WITH XMLNAMESPACES ('urn:test' AS ns)
-SELECT 1 AS [ns:Value]
-FOR XML PATH('ns:Root');"#;
-
- ms().parse_sql_statements(sql).unwrap();
+ ms().verified_stmt("WITH XMLNAMESPACES ('urn:test' AS ns) SELECT 1 AS [ns:Value] FOR XML PATH('ns:Root')");
Review Comment:
merged tests and used verifies_stmt() @iffyio
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3728497045
##########
tests/test_xmlnamespace_integration.rs:
##########
@@ -0,0 +1,133 @@
+/// Test to verify XMLNAMESPACES parsing and AST storage
Review Comment:
you are correct, i removed the file.
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5599276367
> > setting this to draft since its no longer pending review
>
> Is there an expectation for it to be merged?
not at the current diff I don't think it should be merged, please let me know if something's unclear about the unadressed comment I linked to earlier
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3681327534
##########
tests/test_xmlnamespace_integration.rs:
##########
@@ -0,0 +1,133 @@
+/// Test to verify XMLNAMESPACES parsing and AST storage
Review Comment:
hmm I don't think we should introduce a new file for this test. Also let's merge the tests and used either verified_stmt or one_statement_parses_to in tests as I mentioned in the previous review. one simplification of the tests introduced here is to drop the AST assertions, the PR doesn't introduce a new node so its overkill to have each test expiclitly assert the full AST. please have the tests follow existing conventions (this is introducing some println and manual display assertions patterns and is unclear why that's needed)
##########
src/ast/query.rs:
##########
@@ -756,11 +756,23 @@ pub struct With {
pub recursive: bool,
/// The list of CTEs declared by this `WITH` clause.
pub cte_tables: Vec<Cte>,
+ /// Optional XML namespace definitions (`WITH XMLNAMESPACES (...)`).
+ pub xml_namespaces: Vec<XmlNamespaceDefinition>,
Review Comment:
this feature looks similar to what was [done here](https://github.com/apache/datafusion-sqlparser-rs/pull/2333/changes) for clickhouse CSEs, such that I'm thinking we essentially want to introduce this feature in that style instead. is xml_namespaces looks like a regular expression (CSE) so that the enum changes might even be verbatim (similarly for the dialect method name)
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5583652976
> setting this to draft since its no longer pending review
Is there an expectation for it to be merged?
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3674852883
##########
src/parser/mod.rs:
##########
@@ -14108,12 +14108,31 @@ impl<'a> Parser<'a> {
pub fn parse_query(&mut self) -> Result<Box<Query>, ParserError> {
let _guard = self.recursion_counter.try_decrease()?;
let with = if self.parse_keyword(Keyword::WITH) {
- let with_token = self.get_current_token();
- Some(With {
- with_token: with_token.clone().into(),
- recursive: self.parse_keyword(Keyword::RECURSIVE),
- cte_tables: self.parse_comma_separated(Parser::parse_cte)?,
- })
+ let with_token = self.get_current_token().clone();
+ if self.dialect.supports_with_xmlnamespaces_clause()
+ && self.parse_keyword(Keyword::XMLNAMESPACES)
+ {
+ self.expect_token(&Token::LParen)?;
+ let _namespaces =
+ self.parse_comma_separated(Parser::parse_xml_namespace_definition)?;
+ self.expect_token(&Token::RParen)?;
+
+ if self.consume_token(&Token::Comma) {
+ Some(With {
+ with_token: with_token.clone().into(),
+ recursive: self.parse_keyword(Keyword::RECURSIVE),
+ cte_tables: self.parse_comma_separated(Parser::parse_cte)?,
+ })
+ } else {
+ None
Review Comment:
You are correct, I missed the addition to AST fixed now
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5031645584
Hi @iffyio CAn you please help with this PR as well?
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3841446510
##########
src/ast/query.rs:
##########
@@ -756,11 +756,23 @@ pub struct With {
pub recursive: bool,
/// The list of CTEs declared by this `WITH` clause.
pub cte_tables: Vec<Cte>,
+ /// Optional XML namespace definitions (`WITH XMLNAMESPACES (...)`).
+ pub xml_namespaces: Vec<XmlNamespaceDefinition>,
Review Comment:
hmm the latest diff isn't what I meant, to clarify I don't think we should have `pub xml_namespaces: Vec<XmlNamespaceDefinition>` or a new field at all. basically I meant that we implement effectively what's in the linked PR if that absorbs this feature (i.e. the assumption is that xml_namespace is simply a CSE?)
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3664428369
##########
tests/sqlparser_mssql.rs:
##########
@@ -2923,3 +2923,25 @@ fn parse_mssql_money_constants() {
expr_from_projection(only(&select.projection)),
);
}
+
+#[test]
+fn parse_xmlnamespaces() {
+ let sql = r#"WITH XMLNAMESPACES ('urn:test' AS ns)
+SELECT 1 AS [ns:Value]
+FOR XML PATH('ns:Root');"#;
+
+ ms().parse_sql_statements(sql).unwrap();
+
+}
+
+#[test]
+fn parse_xmlnamespaces_with_cte() {
+ let sql = r#"
+WITH XMLNAMESPACES ('urn:example' AS ns), t AS (
+ SELECT 1 AS id
+)
+SELECT id FROM t
+"#;
+
+ ms().parse_sql_statements(sql).unwrap();
+}
Review Comment:
There's a problem that verified_stmt that is using canonical is removing `WITH XMLNAMESPACES ('urn:test' AS ns)` creating only :
```
SELECT 1 AS [ns:Value] FOR XML PATH('ns:Root');
```
causing a fail to the test.
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5565073642
setting this to draft since its no longer pending review
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3674830527
##########
src/dialect/mssql.rs:
##########
@@ -248,6 +248,10 @@ impl Dialect for MsSqlDialect {
_ => None,
}
}
+
+ fn supports_with_xmlnamespaces_clause(&self) -> bool {
Review Comment:
@iffyio Added documentation link
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5550443482
@iffyio I think we can merege :)
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5550379111
@kfirSatori just in case note there's an unadressed comment [here](https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3841446510) on this PR
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5354673159
@iffyio a kind reminder
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5281306946
@iffyio Hi, kind reminder to check or approve :)
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3664428369
##########
tests/sqlparser_mssql.rs:
##########
@@ -2923,3 +2923,25 @@ fn parse_mssql_money_constants() {
expr_from_projection(only(&select.projection)),
);
}
+
+#[test]
+fn parse_xmlnamespaces() {
+ let sql = r#"WITH XMLNAMESPACES ('urn:test' AS ns)
+SELECT 1 AS [ns:Value]
+FOR XML PATH('ns:Root');"#;
+
+ ms().parse_sql_statements(sql).unwrap();
+
+}
+
+#[test]
+fn parse_xmlnamespaces_with_cte() {
+ let sql = r#"
+WITH XMLNAMESPACES ('urn:example' AS ns), t AS (
+ SELECT 1 AS id
+)
+SELECT id FROM t
+"#;
+
+ ms().parse_sql_statements(sql).unwrap();
+}
Review Comment:
There's a problem that verified_stmt that is using canonical is removing `WITH XMLNAMESPACES ('urn:test' AS ns)` creating only :
```
SELECT 1 AS [ns:Value] FOR XML PATH('ns:Root');
```
causing a fail to the test.
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r3621398989
##########
src/parser/mod.rs:
##########
@@ -14108,12 +14108,31 @@ impl<'a> Parser<'a> {
pub fn parse_query(&mut self) -> Result<Box<Query>, ParserError> {
let _guard = self.recursion_counter.try_decrease()?;
let with = if self.parse_keyword(Keyword::WITH) {
- let with_token = self.get_current_token();
- Some(With {
- with_token: with_token.clone().into(),
- recursive: self.parse_keyword(Keyword::RECURSIVE),
- cte_tables: self.parse_comma_separated(Parser::parse_cte)?,
- })
+ let with_token = self.get_current_token().clone();
+ if self.dialect.supports_with_xmlnamespaces_clause()
+ && self.parse_keyword(Keyword::XMLNAMESPACES)
+ {
+ self.expect_token(&Token::LParen)?;
+ let _namespaces =
+ self.parse_comma_separated(Parser::parse_xml_namespace_definition)?;
+ self.expect_token(&Token::RParen)?;
+
+ if self.consume_token(&Token::Comma) {
+ Some(With {
+ with_token: with_token.clone().into(),
+ recursive: self.parse_keyword(Keyword::RECURSIVE),
+ cte_tables: self.parse_comma_separated(Parser::parse_cte)?,
+ })
+ } else {
+ None
Review Comment:
the impl looks odd, is it parsing and dropping the content?
##########
tests/sqlparser_mssql.rs:
##########
@@ -2923,3 +2923,25 @@ fn parse_mssql_money_constants() {
expr_from_projection(only(&select.projection)),
);
}
+
+#[test]
+fn parse_xmlnamespaces() {
+ let sql = r#"WITH XMLNAMESPACES ('urn:test' AS ns)
+SELECT 1 AS [ns:Value]
+FOR XML PATH('ns:Root');"#;
+
+ ms().parse_sql_statements(sql).unwrap();
+
+}
+
+#[test]
+fn parse_xmlnamespaces_with_cte() {
+ let sql = r#"
+WITH XMLNAMESPACES ('urn:example' AS ns), t AS (
+ SELECT 1 AS id
+)
+SELECT id FROM t
+"#;
+
+ ms().parse_sql_statements(sql).unwrap();
+}
Review Comment:
let's merge these and have them use verified_stmt
##########
src/dialect/mssql.rs:
##########
@@ -248,6 +248,10 @@ impl Dialect for MsSqlDialect {
_ => None,
}
}
+
+ fn supports_with_xmlnamespaces_clause(&self) -> bool {
Review Comment:
can we include a link to the documentation of this syntax?
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-5201202885
@iffyio hi,
Can we aprove this please?
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "kfirSatori (via GitHub)" <gi...@apache.org>.
kfirSatori commented on code in PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#discussion_r4218293019
##########
src/ast/query.rs:
##########
@@ -756,11 +756,23 @@ pub struct With {
pub recursive: bool,
/// The list of CTEs declared by this `WITH` clause.
pub cte_tables: Vec<Cte>,
+ /// Optional XML namespace definitions (`WITH XMLNAMESPACES (...)`).
+ pub xml_namespaces: Vec<XmlNamespaceDefinition>,
Review Comment:
Hi @iffyio
I have absorbed the PR and updated my test.
--
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 xmlnamespace support mssql [datafusion-sqlparser-rs]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2361:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2361#issuecomment-6059270065
## [Codecov](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `77.96610%` with `13 lines` in your changes missing coverage. Please review.
:warning: Please [upload](https://docs.codecov.com/docs/codecov-uploader) report for BASE (`main@9296011`). [Learn more](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#section-missing-base-commit) about missing BASE report.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [src/ast/spans.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&filepath=src%2Fast%2Fspans.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL2FzdC9zcGFucy5ycw==) | 64.70% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [src/dialect/mod.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&filepath=src%2Fdialect%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL2RpYWxlY3QvbW9kLnJz) | 50.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [src/parser/mod.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&filepath=src%2Fparser%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL3BhcnNlci9tb2QucnM=) | 85.71% | [0 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [src/ast/query.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&filepath=src%2Fast%2Fquery.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL2FzdC9xdWVyeS5ycw==) | 88.88% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2361 +/- ##
=======================================
Coverage ? 81.11%
=======================================
Files ? 42
Lines ? 33791
Branches ? 33791
=======================================
Hits ? 27411
Misses ? 2812
Partials ? 3568
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2361?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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