diff --git a/src/impls/ast_builder.rs b/src/impls/ast_builder.rs index 5ebfc9ad4..72b5b7bed 100644 --- a/src/impls/ast_builder.rs +++ b/src/impls/ast_builder.rs @@ -4,9 +4,9 @@ use sqlparser::{ ast::{ Assignment, BeginEndStatements, ColumnDef, ConditionalStatements, CreateTable, CreateTableOptions, CreateTrigger, CreateView, Delete, Expr, FromTable, - HiveDistributionStyle, Ident, Insert, ObjectName, Query, SelectItem, Statement, - TableObject, TriggerEvent, TriggerObject, TriggerObjectKind, TriggerPeriod, Update, Values, - ViewColumnDef, helpers::attached_token::AttachedToken, + HiveDistributionStyle, Ident, Insert, ObjectName, Query, SelectItem, SqliteTableOption, + SqliteTableOptions, Statement, TableObject, TriggerEvent, TriggerObject, TriggerObjectKind, + TriggerPeriod, Update, Values, ViewColumnDef, helpers::attached_token::AttachedToken, }, keywords::Keyword, tokenizer::{Token, TokenWithSpan, Word}, @@ -153,7 +153,10 @@ pub(crate) fn create_table( columns, constraints: Vec::new(), if_not_exists, - strict, + sqlite_table_options: SqliteTableOptions { + leading_comma: false, + options: if strict { vec![SqliteTableOption::Strict] } else { Vec::new() }, + }, or_replace: false, temporary: false, unlogged: false, @@ -170,7 +173,6 @@ pub(crate) fn create_table( file_format: None, location: None, query: None, - without_rowid: false, like: None, clone: None, version: None, diff --git a/src/impls/shared_helpers.rs b/src/impls/shared_helpers.rs index ce028b3f2..6a8497398 100644 --- a/src/impls/shared_helpers.rs +++ b/src/impls/shared_helpers.rs @@ -3846,6 +3846,26 @@ pub(crate) fn translate_join_constraint( JoinConstraint::Using(idents) => JoinConstraint::Using(idents.clone()), JoinConstraint::Natural => JoinConstraint::Natural, JoinConstraint::None => JoinConstraint::None, + JoinConstraint::UsingAlias(columns, alias) => { + let clause = format!( + "USING ({}) AS {alias}", + columns.iter().map(ToString::to_string).collect::>().join(", ") + ); + return Err(if D::IS_FORWARD { + Error::forward_refusal(format!( + "the join alias in {clause} names the merged join columns, and SQLite's \ + USING takes no alias. Drop AS {alias} and write a merged column by its bare \ + name, or as coalesce over both joined tables' columns where the bare name is \ + ambiguous. Either table's column alone reads NULL on an outer join's \ + unmatched rows." + )) + } else { + Error::reverse_unsupported_source_syntax(format!( + "{clause} is PostgreSQL grammar that SQLite rejects, so the input is not \ + SQLite. Drop AS {alias}." + )) + }); + } }) } diff --git a/src/impls/translator_impls/create_table.rs b/src/impls/translator_impls/create_table.rs index e0dd4a629..faa7f6df9 100644 --- a/src/impls/translator_impls/create_table.rs +++ b/src/impls/translator_impls/create_table.rs @@ -15,7 +15,8 @@ use alloc::{ use sqlparser::ast::{ ColumnOption, ColumnOptionDef, CreateTable, CreateTableOptions, HiveDistributionStyle, - OnCommit, PrimaryKeyConstraint, TableConstraint, WithData, + OnCommit, PrimaryKeyConstraint, SqliteTableOption, SqliteTableOptions, TableConstraint, + WithData, }; use crate::{ @@ -244,11 +245,14 @@ impl crate::traits::translator::TranslatorWithContext for CreateTable { .collect(), query, // SQLite STRICT mode enforces type checking (not valid on CTAS). - strict: !is_ctas, + // The source carries no table options, refused above. + sqlite_table_options: SqliteTableOptions { + leading_comma: false, + options: if is_ctas { Vec::new() } else { vec![SqliteTableOption::Strict] }, + }, // Legal in both databases and carried through unchanged. temporary: self.temporary, if_not_exists: self.if_not_exists, - without_rowid: self.without_rowid, // Cleared with a warning above when present. unlogged: false, global: None, @@ -361,6 +365,8 @@ impl crate::traits::translator::TranslatorWithContext for CreateTable { fn reject_foreign_create_table_modifiers( create_table: &CreateTable, ) -> Result<(), crate::errors::Error> { + // The one refused clause whose text depends on the input. + let sqlite_options_clause: String; let foreign = if create_table.global.is_some() && !create_table.temporary { // PostgreSQL only accepts GLOBAL and LOCAL immediately before // TEMPORARY, measured on 16. @@ -474,6 +480,14 @@ fn reject_foreign_create_table_modifiers( Some("FALLBACK, which is Teradata") } else if matches!(create_table.with_data, Some(WithData { statistics: Some(_), .. })) { Some("AND STATISTICS, which is Teradata") + } else if !create_table.sqlite_table_options.options.is_empty() { + let options = &create_table.sqlite_table_options.options; + sqlite_options_clause = format!( + "the SQLite table {} {}", + if options.len() == 1 { "option" } else { "options" }, + options.iter().map(ToString::to_string).collect::>().join(", ") + ); + Some(sqlite_options_clause.as_str()) } else { None }; diff --git a/tests/test_ddl_clause_gaps.rs b/tests/test_ddl_clause_gaps.rs index 483e9b9a4..f1999b080 100644 --- a/tests/test_ddl_clause_gaps.rs +++ b/tests/test_ddl_clause_gaps.rs @@ -289,6 +289,26 @@ fn foreign_create_table_modifiers_are_refused() { } } +/// SQLite's table options parse under the PostgreSQL dialect, but PostgreSQL +/// rejects them, so a source carrying one is refused like any other foreign +/// clause, alone or in a list. +#[test] +fn sqlite_table_options_in_the_source_are_refused() { + for (sql, option) in [ + ("CREATE TABLE t (id INT PRIMARY KEY) WITHOUT ROWID;", "WITHOUT ROWID"), + ("CREATE TABLE t (id INT PRIMARY KEY) STRICT;", "STRICT"), + ("CREATE TABLE t (id INT PRIMARY KEY), STRICT;", "STRICT"), + ("CREATE TABLE t (id INT PRIMARY KEY) WITHOUT ROWID, STRICT;", "WITHOUT ROWID, STRICT"), + ] { + let error = translate(sql).expect_err(sql); + let Error::TranslationRefusal(refusal) = error else { + panic!("expected a refusal for {sql}: {error}"); + }; + let detail = refusal.detail(); + assert!(detail.contains(option), "the refusal must name {option}: {detail}"); + } +} + /// `ON COMMIT PRESERVE ROWS` is PostgreSQL's own default for a temporary /// table, and SQLite temporary tables already keep rows across transactions, /// so clearing it is result-neutral and warned. diff --git a/tests/test_forward_queries.rs b/tests/test_forward_queries.rs index 9d8dd76f1..4e14078cd 100644 --- a/tests/test_forward_queries.rs +++ b/tests/test_forward_queries.rs @@ -8,7 +8,7 @@ mod translate_helpers; use diesel::{RunQueryDsl, SqliteConnection, prelude::*}; use helpers::translate_statements; use pg2sqlite::{ - errors::{Error, RefusalCategory}, + errors::{Error, RefusalCategory, TranslationDirection}, options::Pg2SqliteOptions, prelude::{Pg2Sqlite, Translator}, }; @@ -420,6 +420,51 @@ fn order_by_expr_translation_rejects_with_fill_clause() { )); } +/// SQLite's `USING` takes no alias, so PostgreSQL's `USING (...) AS j` is +/// refused wherever the join sits. +#[test] +fn join_using_alias_is_refused_forward() { + for tail in [ + "SELECT j.id FROM users JOIN posts USING (id) AS j;", + "SELECT j.id FROM users LEFT JOIN posts USING (id) AS j;", + "SELECT name FROM users WHERE id IN \ + (SELECT j.id FROM posts JOIN comments USING (id) AS j);", + ] { + let error = Pg2Sqlite::default() + .sql(&format!("{SCHEMA}\n{tail}")) + .unwrap() + .translate(&Pg2SqliteOptions::default()) + .expect_err(tail); + let Error::TranslationRefusal(refusal) = error else { + panic!("expected a refusal for {tail}: {error}"); + }; + assert_eq!(refusal.direction(), TranslationDirection::PostgreSqlToSqlite); + assert_eq!(refusal.category(), RefusalCategory::UnrepresentableSemantics); + assert!(refusal.detail().contains("USING (id) AS j"), "{tail}: {}", refusal.detail()); + } +} + +/// SQLite rejects `USING (...) AS j`, so a SQLite input carrying it is not +/// SQLite and is refused rather than passed on to PostgreSQL. +#[test] +fn join_using_alias_is_refused_reverse() { + let translator = Pg2Sqlite::default().sql(SCHEMA).unwrap(); + let schema = translator.build_schema().unwrap(); + let error = translator + .reverse_sql( + "SELECT j.id FROM users JOIN posts USING (id) AS j", + &schema, + &Pg2SqliteOptions::default(), + ) + .expect_err("SQLite has no join alias"); + let Error::TranslationRefusal(refusal) = error else { + panic!("expected a refusal: {error}"); + }; + assert_eq!(refusal.direction(), TranslationDirection::SqliteToPostgreSql); + assert_eq!(refusal.category(), RefusalCategory::UnsupportedSourceSyntax); + assert!(refusal.detail().contains("USING (id) AS j"), "{}", refusal.detail()); +} + #[test] fn set_expr_insert_translates_expressions() { let options = Pg2SqliteOptions::default();