Skip to content

Commit aefb97b

Browse files
authored
Rework ColumnRef and TableRef (#927)
Extracted the `foo.bar.baz` qualification logic that was duplicated across several variants of `ColumnRef` and `TableRef`. I need this to implement `schema.type`: #827, #922 I'll also need this to implement `schema.function` later: #795 (reply in thread) There are also immediate benefits. IMO, `ColumnRef` and `TableRef` (and their pattern-matching) look considerably cleaner now. The `database.` prefix is guaranteed to be supported everywhere where it may be needed. The detailed changelog is in the diff. I fully preserved (and documented) the existing codegen behavior, as evidenced by unchanged output in tests. If this documented behavior looks suspicious (it kinda does to me), let's fix that later in a separate PR. Here, the only changes in tests are the refactoring that's needed for the new variants to compile. This is a relatively big breaking change for those users who dig within the internals of `ColumnRef`, `TableRef`, and `SchemaTable`. But that's very low-level and should be rare. My work project never does that, despite depending on SeaORM/SeaQuery/SQLx very closely and frequently getting generic or low-level. ## PR Info - Dependents: - #922
1 parent 4e83a97 commit aefb97b

File tree

18 files changed

+279
-237
lines changed

18 files changed

+279
-237
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub struct SeaRc; // new
3131
```
3232
* `impl From<Expr> for Condition`. Now you can use that instead of
3333
`ConditionExpression`, which has been removed.
34+
* Addded `DatabaseName`, `SchemaName`, `TableName`, `ColumnName` types.
3435

3536
### Breaking Changes
3637

@@ -125,6 +126,9 @@ impl Iden for Glyph {
125126
}
126127
}
127128
```
129+
* Reworked `TableRef` and `ColumnRef` variants.
130+
* Turned `SchemaTable` into a type alias of `TableName`. Code that accesses the
131+
fields inside may not compile. Other existing code should still compile.
128132
* Removed `ConditionExpression` from the public API. Instead, just convert
129133
between `Condition` and `Expr` using `From`/`Into`.
130134
* Blanket-implemented `SqliteExpr` and `PgExpr` for `T where T: ExprTrait`.

src/audit/common.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,7 @@ pub(super) fn parse_audit_table(table_ref: &TableRef) -> Option<SchemaTable> {
55
match table_ref {
66
TableRef::SubQuery(_, _) => None,
77
TableRef::FunctionCall(_, _) => None,
8-
TableRef::Table(tbl) | TableRef::TableAlias(tbl, _) => Some(SchemaTable(None, tbl.clone())),
9-
TableRef::SchemaTable(sch, tbl)
10-
| TableRef::DatabaseSchemaTable(_, sch, tbl)
11-
| TableRef::SchemaTableAlias(sch, tbl, _)
12-
| TableRef::DatabaseSchemaTableAlias(_, sch, tbl, _) => {
13-
Some(SchemaTable(Some(sch.clone()), tbl.clone()))
14-
}
8+
TableRef::Table(tbl, _) => Some(tbl.clone()),
159
TableRef::ValuesList(_, _) => None,
1610
}
1711
}

src/audit/mod.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod insert;
44
mod select;
55
mod update;
66

7-
use crate::DynIden;
7+
use crate::{DynIden, TableName};
88

99
pub trait AuditTrait {
1010
fn audit(&self) -> Result<QueryAccessAudit, Error>;
@@ -25,7 +25,10 @@ pub struct QueryAccessAudit {
2525
#[non_exhaustive]
2626
pub struct QueryAccessRequest {
2727
pub access_type: AccessType,
28-
pub schema_table: SchemaTable,
28+
/// Legacy naming, kept for compatibility. It should be `table_name`.
29+
///
30+
/// The table name can be qualified as `(database.)(schema.)table`.
31+
pub schema_table: TableName,
2932
}
3033

3134
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
@@ -47,8 +50,11 @@ pub enum SchemaOper {
4750
Truncate,
4851
}
4952

50-
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
51-
pub struct SchemaTable(pub Option<DynIden>, pub DynIden);
53+
/// A table name, optionally qualified as `(database.)(schema.)table`.
54+
///
55+
/// This is a legacy type alias, to preserve some compatibility.
56+
/// It's going to be deprecated in the future.
57+
pub type SchemaTable = TableName;
5258

5359
impl QueryAccessAudit {
5460
/// This filters the selects from access requests.

src/audit/select.rs

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,10 @@ impl Walker {
8080
match table_ref {
8181
TableRef::SubQuery(select, _) => self.recurse_audit_select(select)?,
8282
TableRef::FunctionCall(function, _) => self.recurse_audit_function(function)?,
83-
TableRef::Table(tbl) | TableRef::TableAlias(tbl, _) => {
83+
TableRef::Table(table_name, _) => {
8484
self.access.push(QueryAccessRequest {
8585
access_type: AccessType::Select,
86-
schema_table: SchemaTable(None, tbl.clone()),
87-
});
88-
}
89-
TableRef::SchemaTable(sch, tbl)
90-
| TableRef::DatabaseSchemaTable(_, sch, tbl)
91-
| TableRef::SchemaTableAlias(sch, tbl, _)
92-
| TableRef::DatabaseSchemaTableAlias(_, sch, tbl, _) => {
93-
self.access.push(QueryAccessRequest {
94-
access_type: AccessType::Select,
95-
schema_table: SchemaTable(Some(sch.clone()), tbl.clone()),
86+
schema_table: table_name.clone(),
9687
});
9788
}
9889
TableRef::ValuesList(_, _) => (),
@@ -179,7 +170,7 @@ impl Walker {
179170
// remove cte alias
180171
for cte in &with_clause.cte_expressions {
181172
if let Some(table_name) = &cte.table_name {
182-
self.remove_item(AccessType::Select, &SchemaTable(None, table_name.clone()));
173+
self.remove_item(AccessType::Select, &TableName(None, table_name.clone()));
183174
}
184175
}
185176
}

src/backend/mysql/foreign_key.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ use super::*;
33
impl ForeignKeyBuilder for MysqlQueryBuilder {
44
fn prepare_table_ref_fk_stmt(&self, table_ref: &TableRef, sql: &mut dyn SqlWriter) {
55
match table_ref {
6-
TableRef::Table(_) => self.prepare_table_ref_iden(table_ref, sql),
6+
// Support only "naked" table names with no schema or alias.
7+
TableRef::Table(TableName(None, _), None) => {
8+
self.prepare_table_ref_iden(table_ref, sql)
9+
}
710
_ => panic!("Not supported"),
811
}
912
}

src/backend/mysql/index.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ impl IndexBuilder for MysqlQueryBuilder {
6060

6161
fn prepare_table_ref_index_stmt(&self, table_ref: &TableRef, sql: &mut dyn SqlWriter) {
6262
match table_ref {
63-
TableRef::Table(_) => self.prepare_table_ref_iden(table_ref, sql),
63+
// Support only "naked" table names with no schema or alias.
64+
TableRef::Table(TableName(None, _), None) => {
65+
self.prepare_table_ref_iden(table_ref, sql)
66+
}
6467
_ => panic!("Not supported"),
6568
}
6669
}

src/backend/mysql/query.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,10 @@ impl QueryBuilder for MysqlQueryBuilder {
9696
self.prepare_iden(column, sql);
9797
} else {
9898
if let Some(table) = table {
99-
if let TableRef::Table(table) = table.deref() {
100-
self.prepare_column_ref(
101-
&ColumnRef::TableColumn(table.clone(), column.clone()),
102-
sql,
103-
);
99+
// Support only "naked" table names with no schema or alias.
100+
if let TableRef::Table(TableName(None, table), None) = table.deref() {
101+
let column_name = ColumnName::from((table.clone(), column.clone()));
102+
self.prepare_column_ref(&ColumnRef::Column(column_name), sql);
104103
return;
105104
}
106105
}

src/backend/postgres/foreign_key.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,8 @@ impl ForeignKeyBuilder for PostgresQueryBuilder {
101101

102102
fn prepare_table_ref_fk_stmt(&self, table_ref: &TableRef, sql: &mut dyn SqlWriter) {
103103
match table_ref {
104-
TableRef::Table(_)
105-
| TableRef::SchemaTable(_, _)
106-
| TableRef::DatabaseSchemaTable(_, _, _) => self.prepare_table_ref_iden(table_ref, sql),
104+
// Support only unaliased (but potentialy qualified) table names.
105+
TableRef::Table(.., None) => self.prepare_table_ref_iden(table_ref, sql),
107106
_ => panic!("Not supported"),
108107
}
109108
}

src/backend/postgres/index.rs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,14 @@ impl IndexBuilder for PostgresQueryBuilder {
7878
}
7979

8080
fn prepare_table_ref_index_stmt(&self, table_ref: &TableRef, sql: &mut dyn SqlWriter) {
81-
match table_ref {
82-
TableRef::Table(_) | TableRef::SchemaTable(_, _) => {
83-
self.prepare_table_ref_iden(table_ref, sql)
84-
}
85-
_ => panic!("Not supported"),
81+
// Support only `table` and `schema.table` forms.
82+
// No `database.schema.table` or aliases.
83+
let TableRef::Table(table_name, None) = table_ref else {
84+
panic!("Not supported");
85+
};
86+
match table_name.as_iden_tuple() {
87+
(Some(_db), _schema, _table) => panic!("Not supported"),
88+
(None, _schema, _table) => self.prepare_table_ref_iden(table_ref, sql),
8689
}
8790
}
8891

@@ -94,13 +97,18 @@ impl IndexBuilder for PostgresQueryBuilder {
9497
}
9598

9699
if let Some(table) = &drop.table {
97-
match table {
98-
TableRef::Table(_) => {}
99-
TableRef::SchemaTable(schema, _) => {
100+
// Support only `table` and `schema.table` forms.
101+
// No `database.schema.table` or aliases.
102+
let TableRef::Table(table_name, None) = table else {
103+
panic!("Not supported");
104+
};
105+
match table_name.as_iden_tuple() {
106+
(None, None, _table) => {}
107+
(None, Some(schema), _table) => {
100108
self.prepare_iden(schema, sql);
101109
write!(sql, ".").unwrap();
102110
}
103-
_ => panic!("Not supported"),
111+
(Some(_db), _schema, _table) => panic!("Not supported"),
104112
}
105113
}
106114
if let Some(name) = &drop.index.name {

src/backend/query_builder.rs

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -596,27 +596,21 @@ pub trait QueryBuilder:
596596

597597
fn prepare_column_ref(&self, column_ref: &ColumnRef, sql: &mut dyn SqlWriter) {
598598
match column_ref {
599-
ColumnRef::Column(column) => self.prepare_iden(column, sql),
600-
ColumnRef::TableColumn(table, column) => {
601-
self.prepare_iden(table, sql);
602-
write!(sql, ".").unwrap();
603-
self.prepare_iden(column, sql);
604-
}
605-
ColumnRef::SchemaTableColumn(schema, table, column) => {
606-
self.prepare_iden(schema, sql);
607-
write!(sql, ".").unwrap();
608-
self.prepare_iden(table, sql);
609-
write!(sql, ".").unwrap();
599+
ColumnRef::Column(ColumnName(table_name, column)) => {
600+
if let Some(table_name) = table_name {
601+
self.prepare_table_name(table_name, sql);
602+
write!(sql, ".").unwrap();
603+
}
610604
self.prepare_iden(column, sql);
611605
}
612-
ColumnRef::Asterisk => {
606+
ColumnRef::Asterisk(table_name) => {
607+
if let Some(table_name) = table_name {
608+
self.prepare_table_name(table_name, sql);
609+
write!(sql, ".").unwrap();
610+
}
613611
write!(sql, "*").unwrap();
614612
}
615-
ColumnRef::TableAsterisk(table) => {
616-
self.prepare_iden(table, sql);
617-
write!(sql, ".*").unwrap();
618-
}
619-
};
613+
}
620614
}
621615

622616
/// Translate [`UnOper`] into SQL statement.

0 commit comments

Comments
 (0)