Skip to content

Commit d17779e

Browse files
committed
chore(cubestore): DF upgrade: Disable ident normalization, remove lowercase normalization in some lookups
This is to get ourselves back in line with old pre-DF-upgrade behavior. Maybe, instead, we should force Cube to quote literals in its queries, but suppose we did that: We're working with generated queries. Normalization to lowercase would mean that any unquoted identifiers that have uppercase characters would be a certain bug. This avoids one factor that would require Cube changes and a Cube upgrade in order to use the Cube.
1 parent bb782d0 commit d17779e

File tree

8 files changed

+132
-99
lines changed

8 files changed

+132
-99
lines changed

rust/cubestore/cubestore-sql-tests/src/tests.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7689,10 +7689,10 @@ async fn inline_tables(service: Box<dyn SqlClient>) {
76897689
);
76907690

76917691
let columns = vec![
7692-
Column::new("id".to_string(), ColumnType::Int, 0),
7693-
Column::new("lastname".to_string(), ColumnType::String, 1),
7694-
Column::new("firstname".to_string(), ColumnType::String, 2),
7695-
Column::new("timestamp".to_string(), ColumnType::Timestamp, 3),
7692+
Column::new("ID".to_string(), ColumnType::Int, 0),
7693+
Column::new("LastName".to_string(), ColumnType::String, 1),
7694+
Column::new("FirstName".to_string(), ColumnType::String, 2),
7695+
Column::new("Timestamp".to_string(), ColumnType::Timestamp, 3),
76967696
];
76977697
let rows = vec![
76987698
Row::new(vec![
@@ -7721,7 +7721,7 @@ async fn inline_tables(service: Box<dyn SqlClient>) {
77217721
]),
77227722
];
77237723
let data = Arc::new(DataFrame::new(columns, rows.clone()));
7724-
let inline_tables = vec![InlineTable::new(1000, "persons".to_string(), data)];
7724+
let inline_tables = vec![InlineTable::new(1000, "Persons".to_string(), data)];
77257725

77267726
let context = SqlQueryContext::default().with_inline_tables(&inline_tables);
77277727
let result = service
@@ -7830,9 +7830,9 @@ async fn inline_tables_2x(service: Box<dyn SqlClient>) {
78307830
.unwrap();
78317831

78327832
let columns = vec![
7833-
Column::new("id".to_string(), ColumnType::Int, 0),
7834-
Column::new("last".to_string(), ColumnType::String, 1),
7835-
Column::new("first".to_string(), ColumnType::String, 2),
7833+
Column::new("ID".to_string(), ColumnType::Int, 0),
7834+
Column::new("Last".to_string(), ColumnType::String, 1),
7835+
Column::new("First".to_string(), ColumnType::String, 2),
78367836
];
78377837
let rows = vec![
78387838
Row::new(vec![
@@ -7871,8 +7871,8 @@ async fn inline_tables_2x(service: Box<dyn SqlClient>) {
78717871
let data = Arc::new(DataFrame::new(columns.clone(), rows.clone()));
78727872
let data2 = Arc::new(DataFrame::new(columns.clone(), rows2.clone()));
78737873
let inline_tables = vec![
7874-
InlineTable::new(1000, "persons".to_string(), data),
7875-
InlineTable::new(1001, "persons2".to_string(), data2),
7874+
InlineTable::new(1000, "Persons".to_string(), data),
7875+
InlineTable::new(1001, "Persons2".to_string(), data2),
78767876
];
78777877

78787878
let context = SqlQueryContext::default().with_inline_tables(&inline_tables);

rust/cubestore/cubestore/src/queryplanner/mod.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl QueryPlanner for QueryPlannerImpl {
153153
state.clone(),
154154
);
155155

156-
let query_planner = SqlToRel::new(&schema_provider);
156+
let query_planner = SqlToRel::new_with_options(&schema_provider, sql_to_rel_options());
157157
let mut logical_plan = query_planner.statement_to_plan(statement)?;
158158

159159
// TODO upgrade DF remove
@@ -349,7 +349,7 @@ impl ContextProvider for MetaStoreSchemaProvider {
349349
let table = self
350350
.inline_tables
351351
.iter()
352-
.find(|inline_table| inline_table.name.to_lowercase() == table.as_ref())
352+
.find(|inline_table| inline_table.name == table.as_ref())
353353
.ok_or_else(|| {
354354
DataFusionError::Plan(format!("Inline table {} was not found", name))
355355
})?;
@@ -574,6 +574,17 @@ impl ContextProvider for MetaStoreSchemaProvider {
574574
}
575575
}
576576

577+
/// Enables our options used with `SqlToRel`. Sets `enable_ident_normalization` to false. See also
578+
/// `normalize_for_column_name` and its doc-comment, and similar functions, which must be kept in
579+
/// sync with changes to the `enable_ident_normalization` option set here.
580+
pub fn sql_to_rel_options() -> datafusion::sql::planner::ParserOptions {
581+
// not to be confused with sql_parser's ParserOptions
582+
datafusion::sql::planner::ParserOptions {
583+
enable_ident_normalization: false,
584+
..Default::default()
585+
}
586+
}
587+
577588
#[derive(Clone, Debug)]
578589
pub enum InfoSchemaTable {
579590
Columns,
@@ -959,7 +970,7 @@ pub mod tests {
959970
other => panic!("not a statement, actual {:?}", other),
960971
};
961972

962-
let plan = SqlToRel::new(&ctx)
973+
let plan = SqlToRel::new_with_options(&ctx, sql_to_rel_options())
963974
.statement_to_plan(DFStatement::Statement(Box::new(statement)))
964975
.unwrap();
965976
SessionContext::new().state().optimize(&plan).unwrap()

rust/cubestore/cubestore/src/queryplanner/partition_filter.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,7 @@ impl Builder<'_> {
575575
#[cfg(test)]
576576
mod tests {
577577
use super::*;
578+
use crate::queryplanner::sql_to_rel_options;
578579
use crate::sql::parser::{CubeStoreParser, Statement as CubeStatement};
579580
use datafusion::arrow::datatypes::Field;
580581
use datafusion::common::{TableReference, ToDFSchema};
@@ -1472,9 +1473,9 @@ mod tests {
14721473
_ => panic!("unexpected parse result"),
14731474
}
14741475

1475-
SqlToRel::new(&NoContextProvider {
1476+
SqlToRel::new_with_options(&NoContextProvider {
14761477
config_options: ConfigOptions::new(),
1477-
})
1478+
}, sql_to_rel_options())
14781479
.sql_to_expr(
14791480
sql_expr,
14801481
&schema.clone().to_dfschema().unwrap(),

0 commit comments

Comments
 (0)