Merge 'core/translate: allow creating column called 'rowid'' from Preston Thorpe

closes #3282
includes minor refactor, removing `column_is_rowid_alias`, which is only
checking the public field of the argument Column.

Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com>

Closes #3385

core/schema.rs

@@ -976,17 +976,13 @@ impl BTreeTable {
     pub fn get_rowid_alias_column(&self) -> Option<(usize, &Column)> {
         if self.primary_key_columns.len() == 1 {
             let (idx, col) = self.get_column(&self.primary_key_columns[0].0)?;
-            if self.column_is_rowid_alias(col) {
+            if col.is_rowid_alias {
                 return Some((idx, col));
             }
         }
         None
     }
 
-    pub fn column_is_rowid_alias(&self, col: &Column) -> bool {
-        col.is_rowid_alias
-    }
-
     /// Returns the column position and column for a given column name.
     /// Returns None if the column name is not found.
     /// E.g. if table is CREATE TABLE t (a, b, c)
@@ -2027,7 +2023,7 @@ mod tests {
         let table = BTreeTable::from_sql(sql, 0)?;
         let column = table.get_column("a").unwrap().1;
         assert!(
-            !table.column_is_rowid_alias(column),
+            !column.is_rowid_alias,
             "column 'a´ has type different than INTEGER so can't be a rowid alias"
         );
         Ok(())
@@ -2038,10 +2034,7 @@ mod tests {
         let sql = r#"CREATE TABLE t1 (a INTEGER PRIMARY KEY, b TEXT);"#;
         let table = BTreeTable::from_sql(sql, 0)?;
         let column = table.get_column("a").unwrap().1;
-        assert!(
-            table.column_is_rowid_alias(column),
-            "column 'a´ should be a rowid alias"
-        );
+        assert!(column.is_rowid_alias, "column 'a´ should be a rowid alias");
         Ok(())
     }
 
@@ -2051,10 +2044,7 @@ mod tests {
         let sql = r#"CREATE TABLE t1 (a INTEGER, b TEXT, PRIMARY KEY(a));"#;
         let table = BTreeTable::from_sql(sql, 0)?;
         let column = table.get_column("a").unwrap().1;
-        assert!(
-            table.column_is_rowid_alias(column),
-            "column 'a´ should be a rowid alias"
-        );
+        assert!(column.is_rowid_alias, "column 'a´ should be a rowid alias");
         Ok(())
     }
 
@@ -2065,7 +2055,7 @@ mod tests {
         let table = BTreeTable::from_sql(sql, 0)?;
         let column = table.get_column("a").unwrap().1;
         assert!(
-            !table.column_is_rowid_alias(column),
+            !column.is_rowid_alias,
             "column 'a´ shouldn't be a rowid alias because table has no rowid"
         );
         Ok(())
@@ -2077,7 +2067,7 @@ mod tests {
         let table = BTreeTable::from_sql(sql, 0)?;
         let column = table.get_column("a").unwrap().1;
         assert!(
-            !table.column_is_rowid_alias(column),
+            !column.is_rowid_alias,
             "column 'a´ shouldn't be a rowid alias because table has no rowid"
         );
         Ok(())
@@ -2100,7 +2090,7 @@ mod tests {
         let table = BTreeTable::from_sql(sql, 0)?;
         let column = table.get_column("a").unwrap().1;
         assert!(
-            !table.column_is_rowid_alias(column),
+            !column.is_rowid_alias,
             "column 'a´ shouldn't be a rowid alias because table has composite primary key"
         );
         Ok(())

core/translate/expr.rs

@@ -3302,12 +3302,6 @@ pub fn bind_and_rewrite_expr<'a>(
         top_level_expr,
         &mut |expr: &mut ast::Expr| -> Result<WalkControl> {
             match expr {
-                ast::Expr::Id(ast::Name::Ident(n)) if n.eq_ignore_ascii_case("true") => {
-                    *expr = ast::Expr::Literal(ast::Literal::Numeric("1".to_string()));
-                }
-                ast::Expr::Id(ast::Name::Ident(n)) if n.eq_ignore_ascii_case("false") => {
-                    *expr = ast::Expr::Literal(ast::Literal::Numeric("0".to_string()));
-                }
                 // Rewrite anonymous variables in encounter order.
                 ast::Expr::Variable(var) if var.is_empty() => {
                     if !param_state.is_valid() {
@@ -3370,17 +3364,6 @@ pub fn bind_and_rewrite_expr<'a>(
                                 }
                             }
                         }
-                        if !referenced_tables.joined_tables().is_empty() {
-                            if let Some(row_id_expr) = parse_row_id(
-                                &normalized_id,
-                                referenced_tables.joined_tables()[0].internal_id,
-                                || referenced_tables.joined_tables().len() != 1,
-                            )? {
-                                *expr = row_id_expr;
-
-                                return Ok(WalkControl::Continue);
-                            }
-                        }
                         let mut match_result = None;
 
                         // First check joined tables
@@ -3416,6 +3399,15 @@ pub fn bind_and_rewrite_expr<'a>(
                                     col_idx.unwrap(),
                                     col.is_rowid_alias,
                                 ));
+                            // only if we haven't found a match, check for explicit rowid reference
+                            } else if let Some(row_id_expr) = parse_row_id(
+                                &normalized_id,
+                                referenced_tables.joined_tables()[0].internal_id,
+                                || referenced_tables.joined_tables().len() != 1,
+                            )? {
+                                *expr = row_id_expr;
+
+                                return Ok(WalkControl::Continue);
                             }
                         }
 
@@ -3496,17 +3488,16 @@ pub fn bind_and_rewrite_expr<'a>(
                         }
                         let (tbl_id, tbl) = matching_tbl.unwrap();
                         let normalized_id = normalize_ident(id.as_str());
-
-                        if let Some(row_id_expr) = parse_row_id(&normalized_id, tbl_id, || false)? {
-                            *expr = row_id_expr;
-
-                            return Ok(WalkControl::Continue);
-                        }
                         let col_idx = tbl.columns().iter().position(|c| {
                             c.name
                                 .as_ref()
                                 .is_some_and(|name| name.eq_ignore_ascii_case(&normalized_id))
                         });
+                        if let Some(row_id_expr) = parse_row_id(&normalized_id, tbl_id, || false)? {
+                            *expr = row_id_expr;
+
+                            return Ok(WalkControl::Continue);
+                        }
                         let Some(col_idx) = col_idx else {
                             crate::bail_parse_error!("no such column: {}", normalized_id);
                         };

core/translate/update.rs

@@ -228,36 +228,40 @@ pub fn prepare_update_plan(
         for (col_name, expr) in set.col_names.iter().zip(values.iter()) {
             let ident = normalize_ident(col_name.as_str());
 
-            // Check if this is the 'rowid' keyword
-            if ROWID_STRS.iter().any(|s| s.eq_ignore_ascii_case(&ident)) {
-                // Find the rowid alias column if it exists
-                if let Some((idx, _col)) = table
-                    .columns()
-                    .iter()
-                    .enumerate()
-                    .find(|(_, c)| c.is_rowid_alias)
-                {
-                    // Use the rowid alias column index
-                    match set_clauses.iter_mut().find(|(i, _)| i == &idx) {
-                        Some((_, existing_expr)) => *existing_expr = expr.clone(),
-                        None => set_clauses.push((idx, expr.clone())),
-                    }
-                } else {
-                    // No rowid alias, use sentinel value for actual rowid
-                    match set_clauses.iter_mut().find(|(i, _)| *i == ROWID_SENTINEL) {
-                        Some((_, existing_expr)) => *existing_expr = expr.clone(),
-                        None => set_clauses.push((ROWID_SENTINEL, expr.clone())),
+            let col_index = match column_lookup.get(&ident) {
+                Some(idx) => *idx,
+                None => {
+                    // Check if this is the 'rowid' keyword
+                    if ROWID_STRS.iter().any(|s| s.eq_ignore_ascii_case(&ident)) {
+                        // Find the rowid alias column if it exists
+                        if let Some((idx, _col)) = table
+                            .columns()
+                            .iter()
+                            .enumerate()
+                            .find(|(_i, c)| c.is_rowid_alias)
+                        {
+                            // Use the rowid alias column index
+                            match set_clauses.iter_mut().find(|(i, _)| i == &idx) {
+                                Some((_, existing_expr)) => *existing_expr = expr.clone(),
+                                None => set_clauses.push((idx, expr.clone())),
+                            }
+                            idx
+                        } else {
+                            // No rowid alias, use sentinel value for actual rowid
+                            match set_clauses.iter_mut().find(|(i, _)| *i == ROWID_SENTINEL) {
+                                Some((_, existing_expr)) => *existing_expr = expr.clone(),
+                                None => set_clauses.push((ROWID_SENTINEL, expr.clone())),
+                            }
+                            ROWID_SENTINEL
+                        }
+                    } else {
+                        crate::bail_parse_error!("no such column: {}.{}", table_name, col_name);
                     }
                 }
-            } else {
-                let col_index = match column_lookup.get(&ident) {
-                    Some(idx) => idx,
-                    None => bail_parse_error!("no such column: {}", ident),
-                };
-                match set_clauses.iter_mut().find(|(idx, _)| idx == col_index) {
-                    Some((_, existing_expr)) => *existing_expr = expr.clone(),
-                    None => set_clauses.push((*col_index, expr.clone())),
-                }
+            };
+            match set_clauses.iter_mut().find(|(idx, _)| *idx == col_index) {
+                Some((_, existing_expr)) => *existing_expr = expr.clone(),
+                None => set_clauses.push((col_index, expr.clone())),
             }
         }
     }

parser/src/parser.rs

@@ -3446,10 +3446,6 @@ impl<'a> Parser<'a> {
 
     pub fn parse_column_definition(&mut self, in_alter: bool) -> Result<ColumnDefinition> {
         let col_name = self.parse_nm()?;
-        if !in_alter && col_name.as_str().eq_ignore_ascii_case("rowid") {
-            return Err(Error::Custom("cannot use reserved word: ROWID".to_owned()));
-        }
-
         let col_type = self.parse_type()?;
         let constraints = self.parse_named_column_constraints(in_alter)?;
         Ok(ColumnDefinition {
@@ -4039,7 +4035,6 @@ mod tests {
             "ALTER TABLE my_table ADD COLUMN my_column PRIMARY KEY",
             "ALTER TABLE my_table ADD COLUMN my_column UNIQUE",
             "CREATE TEMP TABLE baz.foo(bar)",
-            "CREATE TABLE foo(rowid)",
             "CREATE TABLE foo(d INT AS (a*abs(b)))",
             "CREATE TABLE foo(d INT AS (a*abs(b)))",
             "CREATE TABLE foo(bar UNKNOWN_INT) STRICT",

testing/create_table.test

@@ -45,3 +45,11 @@ do_execsql_test_in_memory_any_error create_table_column_and_table_primary_keys {
 do_execsql_test_in_memory_any_error create_table_multiple_table_primary_keys {
     CREATE TABLE t(a,b,c,d,primary key(a,b), primary key(c,d));
 }
+
+# https://github.com/tursodatabase/turso/issues/3282
+do_execsql_test_on_specific_db {:memory:} col-named-rowid {
+  create table t(rowid, a);
+  insert into t values (1,2), (2,3), (3,4);
+  update t set rowid = 1; -- should allow regular update and not throw unique constraint
+  select count(*) from t where rowid = 1;
+} {3}