sabir-akhadov-localstack commented on code in PR #2457:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2457#discussion_r4323105385


##########
src/dialect/mod.rs:
##########
@@ -1208,8 +1245,11 @@ pub trait Dialect: Debug + Any {
         false
     }
 
-    /// Returns true if this dialect allows dollar placeholders
-    /// e.g. `SELECT $var` (SQLite)
+    /// Returns true if this dialect allows SQLite-style named dollar 
placeholders,
+    /// for example `SELECT $name`.
+    ///
+    /// This does not describe PostgreSQL-style ordered binds such as `SELECT 
$1`.

Review Comment:
   This hook does not gate `$name` placeholders: `$name` and `$1` tokenize as 
`Placeholder` in every dialect (see the `$1` tests in sqlparser_postgres.rs and 
`$123` in sqlparser_mssql.rs, both with the flag off). What it actually toggles 
is whether `$$…$$` / `$tag$…$tag$` is dollar-quoting (false) or a placeholder 
with `$` allowed inside (true, SQLite); the doc should say that.



##########
src/dialect/postgresql.rs:
##########
@@ -77,6 +77,10 @@ impl Dialect for PostgreSqlDialect {
         Some('"')
     }
 
+    fn ordered_bind_placeholder_style(&self) -> Option<BindPlaceholderStyle> {
+        Some(BindPlaceholderStyle::DollarNumbered)

Review Comment:
   Redshift uses the same `$1` positional syntax 
(https://docs.aws.amazon.com/redshift/latest/dg/r_PREPARE.html, "refer to the 
parameters by position using the notation $1, $2"), so `RedshiftSqlDialect` 
should return `DollarNumbered` too rather than falling through to `None`.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to