Skip to content

Commit

Permalink
Don't name anonymous columns too soon
Browse files Browse the repository at this point in the history
This helps us handle some pretty tricky cases.
  • Loading branch information
emk committed Oct 30, 2023
1 parent aa634b7 commit fcfb2b0
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 37 deletions.
35 changes: 10 additions & 25 deletions src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,17 +116,15 @@ impl InferTypes for ast::CreateTableStatement {
// TODO: We don't support this in the main grammar yet.
not_null: false,
};
col_ty.expect_creatable(column)?;
Ok(col_ty)
})
.collect::<Result<Vec<_>>>()?;

scope.add(
ident_from_table_name(table_name)?,
Type::Table(TableType {
columns: column_decls,
}),
)?;
let table_type = TableType {
columns: column_decls,
}
.name_anonymous_columns(table_name.span());
table_type.expect_creatable(table_name)?;
scope.add(ident_from_table_name(table_name)?, Type::Table(table_type))?;
Ok(((), scope.into_handle()))
}
ast::CreateTableStatement {
Expand All @@ -138,9 +136,8 @@ impl InferTypes for ast::CreateTableStatement {
..
} => {
let (ty, _scope) = query_statement.infer_types(scope)?;
for col_ty in &ty.columns {
col_ty.expect_creatable(query_statement)?;
}
let ty = ty.name_anonymous_columns(table_name.span());
ty.expect_creatable(table_name)?;
let mut scope = Scope::new(scope);
scope.add(ident_from_table_name(table_name)?, Type::Table(ty))?;
Ok(((), scope.into_handle()))
Expand Down Expand Up @@ -290,32 +287,20 @@ impl InferTypes for ast::SelectExpression {
((), scope) = from_clause.infer_types(&scope)?;
}

let mut next_anon_col_id: u64 = 0;
let mut cols = vec![];
for item in select_list.node_iter_mut() {
match item {
ast::SelectListItem::Expression { expression, alias } => {
// BigQuery does not allow select list items to see names
// bound by other select list items.
//
// TODO: Delay assigning anonymous names until we actually
// create a table?
let (ty, _scope) = expression.infer_types(&scope)?;
// Make sure any aggregates have been turned into values.
let ty = ty.expect_value_type(expression)?.to_owned();
let name = alias
.infer_column_name()
.or_else(|| expression.infer_column_name())
.unwrap_or_else(|| {
// We try to predict how BigQuery will name these.
let ident =
Ident::new(&format!("_f{}", next_anon_col_id), expression.span());
next_anon_col_id += 1;
ident
});

.or_else(|| expression.infer_column_name());
cols.push(ColumnType {
name: Some(name),
name,
ty: ArgumentType::Value(ty),
not_null: false,
});
Expand Down
47 changes: 35 additions & 12 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,37 @@ impl TableType {
}
Ok(())
}

/// Expect this table to be creatable, i.e., that it contains no aggregate
/// columns or uninhabited types.
pub fn expect_creatable(&self, spanned: &dyn Spanned) -> Result<()> {
for column in &self.columns {
column.expect_creatable(spanned)?;
}
Ok(())
}

/// Name all anonymous columns in this table, trying to mimic BigQuery's
/// behavior. BigQuery names anonymous columns `_f0`, `_f1`, etc., but
/// only increments the counter for anonymous columns, not all columns.
pub fn name_anonymous_columns(&self, span: Span) -> Self {
let mut counter = 0;
let mut new_columns = Vec::new();
for column in &self.columns {
if column.name.is_none() {
new_columns.push(ColumnType {
name: Some(Ident::new(&format!("_f{}", counter), span.clone())),
..column.clone()
});
counter += 1;
} else {
new_columns.push(column.clone());
}
}
TableType {
columns: new_columns,
}
}
}

impl fmt::Display for TableType {
Expand Down Expand Up @@ -807,20 +838,12 @@ pub struct ColumnType {
}

impl ColumnType {
/// Return an error if this column is not storable.
/// Return an error if this column is not storable. We do not check to see
/// if the column is named. You can fix that by calling
/// [`TableType::name_anonymous_columns`].
pub fn expect_creatable(&self, spanned: &dyn Spanned) -> Result<()> {
match &self.ty {
ArgumentType::Value(ty) => {
ty.expect_inhabited(spanned)?;
if self.name.is_none() {
return Err(Error::annotated(
"cannot store a table with anonymous columns",
spanned.span(),
"type mismatch",
));
}
Ok(())
}
ArgumentType::Value(ty) => ty.expect_inhabited(spanned),
ArgumentType::Aggregating(_) => Err(Error::annotated(
"Cannot store an aggregate column",
spanned.span(),
Expand Down

0 comments on commit fcfb2b0

Please sign in to comment.