Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: change IGNORE COLUMNS MySQL option to EXCLUDE COLUMNS #29438

Merged
merged 1 commit into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion doc/user/content/sql/create-source/mysql.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ _src_name_ | The name for the source.

Field | Value | Description
-------------------------------------|---------------------------------|-------------------------------------
`IGNORE COLUMNS` | A list of fully-qualified names | Ignore specific columns that cannot be decoded or should not be included in the subsources created in Materialize.
{{< if-unreleased "v0.117" >}}`IGNORE COLUMNS`{{< /if-unreleased >}} {{< if-released "v0.117" >}}`EXCLUDE COLUMNS`{{< /if-released >}} | A list of fully-qualified names | Exclude specific columns that cannot be decoded or should not be included in the subsources created in Materialize.
`TEXT COLUMNS` | A list of fully-qualified names | Decode data as `text` for specific columns that contain MySQL types that are [unsupported in Materialize](#supported-types).

## Features
Expand Down Expand Up @@ -256,9 +256,17 @@ following types:
<li><code>year</code></li>
</ul>

{{< if-unreleased "v0.117" >}}
The specified columns will be treated as `text`, and will thus not offer the
expected MySQL type features. For any unsupported data types not listed above,
use the [`IGNORE COLUMNS`](#ignoring-columns) option.
{{< /if-unreleased >}}

{{< if-released "v0.117" >}}
The specified columns will be treated as `text`, and will thus not offer the
expected MySQL type features. For any unsupported data types not listed above,
use the [`EXCLUDE COLUMNS`](#excluding-columns) option.
{{< /if-released >}}

##### Truncation

Expand Down Expand Up @@ -372,6 +380,7 @@ CREATE SOURCE mz_source
FOR ALL TABLES;
```

{{< if-unreleased "v0.117" >}}
#### Ignoring columns

MySQL doesn't provide a way to filter out columns from the replication stream.
Expand All @@ -385,6 +394,23 @@ CREATE SOURCE mz_source
)
FOR ALL TABLES;
```
{{< /if-unreleased >}}

{{< if-released "v0.117" >}}
#### Excluding columns

MySQL doesn't provide a way to filter out columns from the replication stream.
To exclude specific upstream columns from being ingested, use the `EXCLUDE
COLUMNS` option.

```mzsql
CREATE SOURCE mz_source
FROM MYSQL CONNECTION mysql_connection (
EXCLUDE COLUMNS (mydb.table_1.column_to_ignore)
)
FOR ALL TABLES;
```
{{< /if-released >}}

### Handling errors and schema changes

Expand Down
165 changes: 110 additions & 55 deletions doc/user/layouts/partials/sql-grammar/create-source-mysql.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions doc/user/sql-grammar/sql-grammar.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ create_source_mysql ::=
'FROM' 'MYSQL' 'CONNECTION' connection_name
( ( '(' 'TEXT COLUMNS' ('(' (column_name) ( ( ',' column_name ) )* ')')? )? )
( ( ',' 'IGNORE COLUMNS' ('(' (column_name) ( ( ',' column_name ) )* ')')? ')' )? )
( ( ',' 'EXCLUDE COLUMNS' ('(' (column_name) ( ( ',' column_name ) )* ')')? ')' )? )
Comment on lines 234 to +235
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
( ( ',' 'IGNORE COLUMNS' ('(' (column_name) ( ( ',' column_name ) )* ')')? ')' )? )
( ( ',' 'EXCLUDE COLUMNS' ('(' (column_name) ( ( ',' column_name ) )* ')')? ')' )? )
( ( ',' ('IGNORE COLUMNS' | 'EXCLUDE COLUMNS' ) ('(' (column_name) ( ( ',' column_name ) )* ')')? ')' )? )

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry, I missed this suggestion. Going to merge this while CI is green, but I'll circle back on this next Thursday when I remove the IGNORE COLUMNS from the syntax diagram entirely.

('FOR ALL TABLES'
| 'FOR TABLES' '(' table_name ('AS' subsrc_name)? (',' table_name ('AS' subsrc_name)? )* ')'
| 'FOR SCHEMAS' '(' schema_name (',' schema_name )* ')'
Expand Down
28 changes: 14 additions & 14 deletions src/adapter/src/coord/sequencer/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3631,7 +3631,7 @@ impl Coordinator {

let mz_sql::plan::AlterSourceAddSubsourceOptionExtracted {
text_columns: mut new_text_columns,
ignore_columns: mut new_ignore_columns,
exclude_columns: mut new_exclude_columns,
..
} = options.try_into()?;

Expand Down Expand Up @@ -3709,7 +3709,7 @@ impl Coordinator {
} => {
let mz_sql::plan::MySqlConfigOptionExtracted {
mut text_columns,
mut ignore_columns,
mut exclude_columns,
..
} = curr_options.clone().try_into()?;

Expand All @@ -3719,28 +3719,28 @@ impl Coordinator {
!matches!(
o.name,
MySqlConfigOptionName::TextColumns
| MySqlConfigOptionName::IgnoreColumns
| MySqlConfigOptionName::ExcludeColumns
)
});

// Drop all text / ignore columns that are not currently referred to.
// Drop all text / exclude columns that are not currently referred to.
let column_referenced =
|column_qualified_reference: &UnresolvedItemName| {
mz_ore::soft_assert_eq_or_log!(
column_qualified_reference.0.len(),
3,
"all TEXT COLUMNS & IGNORE COLUMNS values must be column-qualified references"
"all TEXT COLUMNS & EXCLUDE COLUMNS values must be column-qualified references"
);
let mut table = column_qualified_reference.clone();
table.0.truncate(2);
curr_references.contains(&table)
};
text_columns.retain(column_referenced);
ignore_columns.retain(column_referenced);
exclude_columns.retain(column_referenced);

// Merge the current text / ignore columns into the new text / ignore columns.
// Merge the current text / exclude columns into the new text / exclude columns.
new_text_columns.extend(text_columns);
new_ignore_columns.extend(ignore_columns);
new_exclude_columns.extend(exclude_columns);

// If we have text columns, add them to the options.
if !new_text_columns.is_empty() {
Expand All @@ -3755,17 +3755,17 @@ impl Coordinator {
value: Some(WithOptionValue::Sequence(new_text_columns)),
});
}
// If we have ignore columns, add them to the options.
if !new_ignore_columns.is_empty() {
new_ignore_columns.sort();
let new_ignore_columns = new_ignore_columns
// If we have exclude columns, add them to the options.
if !new_exclude_columns.is_empty() {
new_exclude_columns.sort();
let new_exclude_columns = new_exclude_columns
.into_iter()
.map(WithOptionValue::UnresolvedItemName)
.collect();

curr_options.push(MySqlConfigOption {
name: MySqlConfigOptionName::IgnoreColumns,
value: Some(WithOptionValue::Sequence(new_ignore_columns)),
name: MySqlConfigOptionName::ExcludeColumns,
value: Some(WithOptionValue::Sequence(new_exclude_columns)),
});
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/mysql-util/src/schemas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,14 @@ impl MySqlTableSchema {
}

/// Convert the raw table schema to our MySqlTableDesc representation
/// using any provided text_columns and ignore_columns
/// using any provided text_columns and exclude_columns
pub fn to_desc(
self,
text_columns: Option<&BTreeSet<&str>>,
ignore_columns: Option<&BTreeSet<&str>>,
exclude_columns: Option<&BTreeSet<&str>>,
) -> Result<MySqlTableDesc, MySqlError> {
// Verify there are no duplicates in text_columns and ignore_columns
match (&text_columns, &ignore_columns) {
// Verify there are no duplicates in text_columns and exclude_columns
match (&text_columns, &exclude_columns) {
(Some(text_cols), Some(ignore_cols)) => {
let intersection: Vec<_> = text_cols.intersection(ignore_cols).collect();
if !intersection.is_empty() {
Expand Down Expand Up @@ -133,7 +133,7 @@ impl MySqlTableSchema {
}

// If this column is ignored, use None for the column type to signal that it should be.
if let Some(ignore_cols) = &ignore_columns {
if let Some(ignore_cols) = &exclude_columns {
if ignore_cols.contains(&info.column_name.as_str()) {
columns.push(MySqlColumnDesc {
name: info.column_name,
Expand Down
1 change: 1 addition & 0 deletions src/sql-lexer/src/keywords.txt
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ Escape
Estimate
Every
Except
Exclude
Execute
Exists
Expected
Expand Down
8 changes: 4 additions & 4 deletions src/sql-parser/src/ast/defs/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1092,21 +1092,21 @@ pub enum MySqlConfigOptionName {
/// fully deprecating that feature and forcing users to use explicit
/// `CREATE TABLE .. FROM SOURCE` statements
TextColumns,
/// Columns you want to ignore
/// Columns you want to exclude
/// NOTE(roshan): This value is kept around to allow round-tripping a
/// `CREATE SOURCE` statement while we still allow creating implicit
/// subsources from `CREATE SOURCE`, but will be removed once
/// fully deprecating that feature and forcing users to use explicit
/// `CREATE TABLE .. FROM SOURCE` statements
IgnoreColumns,
ExcludeColumns,
}

impl AstDisplay for MySqlConfigOptionName {
fn fmt<W: fmt::Write>(&self, f: &mut AstFormatter<W>) {
f.write_str(match self {
MySqlConfigOptionName::Details => "DETAILS",
MySqlConfigOptionName::TextColumns => "TEXT COLUMNS",
MySqlConfigOptionName::IgnoreColumns => "IGNORE COLUMNS",
MySqlConfigOptionName::ExcludeColumns => "EXCLUDE COLUMNS",
})
}
}
Expand All @@ -1122,7 +1122,7 @@ impl WithOptionName for MySqlConfigOptionName {
match self {
MySqlConfigOptionName::Details
| MySqlConfigOptionName::TextColumns
| MySqlConfigOptionName::IgnoreColumns => false,
| MySqlConfigOptionName::ExcludeColumns => false,
}
}
}
Expand Down
22 changes: 11 additions & 11 deletions src/sql-parser/src/ast/defs/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1098,8 +1098,8 @@ pub enum CreateSubsourceOptionName {
ExternalReference,
/// Columns whose types you want to unconditionally format as text
TextColumns,
/// Columns you want to ignore when ingesting data
IgnoreColumns,
/// Columns you want to exclude when ingesting data
ExcludeColumns,
/// `DETAILS` for this subsource, hex-encoded protobuf type
/// `mz_storage_types::sources::SourceExportStatementDetails`
Details,
Expand All @@ -1111,7 +1111,7 @@ impl AstDisplay for CreateSubsourceOptionName {
CreateSubsourceOptionName::Progress => "PROGRESS",
CreateSubsourceOptionName::ExternalReference => "EXTERNAL REFERENCE",
CreateSubsourceOptionName::TextColumns => "TEXT COLUMNS",
CreateSubsourceOptionName::IgnoreColumns => "IGNORE COLUMNS",
CreateSubsourceOptionName::ExcludeColumns => "EXCLUDE COLUMNS",
CreateSubsourceOptionName::Details => "DETAILS",
})
}
Expand All @@ -1129,7 +1129,7 @@ impl WithOptionName for CreateSubsourceOptionName {
| CreateSubsourceOptionName::ExternalReference
| CreateSubsourceOptionName::Details
| CreateSubsourceOptionName::TextColumns
| CreateSubsourceOptionName::IgnoreColumns => false,
| CreateSubsourceOptionName::ExcludeColumns => false,
}
}
}
Expand Down Expand Up @@ -1510,8 +1510,8 @@ impl_display_for_with_option!(TableOption);
pub enum TableFromSourceOptionName {
/// Columns whose types you want to unconditionally format as text
TextColumns,
/// Columns you want to ignore when ingesting data
IgnoreColumns,
/// Columns you want to exclude when ingesting data
ExcludeColumns,
/// Hex-encoded protobuf of a `ProtoSourceExportStatementDetails`
/// message, which includes details necessary for planning this
/// table as a Source Export
Expand All @@ -1522,7 +1522,7 @@ impl AstDisplay for TableFromSourceOptionName {
fn fmt<W: fmt::Write>(&self, f: &mut AstFormatter<W>) {
f.write_str(match self {
TableFromSourceOptionName::TextColumns => "TEXT COLUMNS",
TableFromSourceOptionName::IgnoreColumns => "IGNORE COLUMNS",
TableFromSourceOptionName::ExcludeColumns => "EXCLUDE COLUMNS",
TableFromSourceOptionName::Details => "DETAILS",
})
}
Expand All @@ -1539,7 +1539,7 @@ impl WithOptionName for TableFromSourceOptionName {
match self {
TableFromSourceOptionName::Details
| TableFromSourceOptionName::TextColumns
| TableFromSourceOptionName::IgnoreColumns => false,
| TableFromSourceOptionName::ExcludeColumns => false,
}
}
}
Expand Down Expand Up @@ -2534,7 +2534,7 @@ pub enum AlterSourceAddSubsourceOptionName {
/// Columns whose types you want to unconditionally format as text
TextColumns,
/// Columns you want to ignore when ingesting data
IgnoreColumns,
ExcludeColumns,
/// Updated `DETAILS` for an ingestion, e.g.
/// [`crate::ast::PgConfigOptionName::Details`]
/// or
Expand All @@ -2546,7 +2546,7 @@ impl AstDisplay for AlterSourceAddSubsourceOptionName {
fn fmt<W: fmt::Write>(&self, f: &mut AstFormatter<W>) {
f.write_str(match self {
AlterSourceAddSubsourceOptionName::TextColumns => "TEXT COLUMNS",
AlterSourceAddSubsourceOptionName::IgnoreColumns => "IGNORE COLUMNS",
AlterSourceAddSubsourceOptionName::ExcludeColumns => "EXCLUDE COLUMNS",
AlterSourceAddSubsourceOptionName::Details => "DETAILS",
})
}
Expand All @@ -2563,7 +2563,7 @@ impl WithOptionName for AlterSourceAddSubsourceOptionName {
match self {
AlterSourceAddSubsourceOptionName::Details
| AlterSourceAddSubsourceOptionName::TextColumns
| AlterSourceAddSubsourceOptionName::IgnoreColumns => false,
| AlterSourceAddSubsourceOptionName::ExcludeColumns => false,
}
}
}
Expand Down
Loading
Loading