Skip to content

Commit

Permalink
adapter: Delete migration for avg_internal_v1 (#21848)
Browse files Browse the repository at this point in the history
Removes the migration introduced as part of
#21219

### Motivation

* This PR refactors existing code.
  Cleans up a migration I've been meaning to delete

### Checklist

- [ ] This PR has adequate test coverage / QA involvement has been duly
considered.
- [ ] This PR has an associated up-to-date [design
doc](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/README.md),
is a design doc
([template](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/design/00000000_template.md)),
or is sufficiently small to not require a design.
  <!-- Reference the design in the description. -->
- [ ] If this PR evolves [an existing `$T ⇔ Proto$T`
mapping](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/command-and-response-binary-encoding.md)
(possibly in a backwards-incompatible way), then it is tagged with a
`T-proto` label.
- [ ] If this PR will require changes to cloud orchestration or tests,
there is a companion cloud PR to account for those changes that is
tagged with the release-blocker label
([example](MaterializeInc/cloud#5021)).
<!-- Ask in #team-cloud on Slack if you need help preparing the cloud
PR. -->
- [x] This PR includes the following [user-facing behavior
changes](https://github.com/MaterializeInc/materialize/blob/main/doc/developer/guide-changes.md#what-changes-require-a-release-note):
  - N/a
  • Loading branch information
ParkMyCar authored Sep 20, 2023
1 parent d45c4ce commit 4460d47
Showing 1 changed file with 0 additions and 63 deletions.
63 changes: 0 additions & 63 deletions src/adapter/src/catalog/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,12 @@ use mz_ore::collections::CollectionExt;
use mz_ore::now::EpochMillis;
use mz_sql::ast::display::AstDisplay;
use mz_sql::ast::Raw;
use mz_sql_parser::ast::visit_mut::VisitMut;
use mz_sql_parser::ast::{Function, Ident};
use mz_storage_types::connections::ConnectionContext;
use semver::Version;
use tracing::info;

use crate::catalog::{Catalog, ConnCatalog};

/// A flag to indicate whether or not we've already run the migration to rename existing uses of
/// the AVG(...) function. Runs in versions <=0.66.
const RENAME_AVG_FUNCTION_MIGRATION_FLAG: &str = "rename_avg_function_migration_flag";

async fn rewrite_items<F>(
tx: &mut Transaction<'_>,
cat: Option<&ConnCatalog<'_>>,
Expand Down Expand Up @@ -72,21 +66,6 @@ pub(crate) async fn migrate(
// First, do basic AST -> AST transformations.
// rewrite_items(&mut tx, None, |_tx, _cat, _stmt| Box::pin(async { Ok(()) })).await?;

// Only run this migration if we haven't run it already.
let avg_function_has_run =
tx.check_migration_has_run(RENAME_AVG_FUNCTION_MIGRATION_FLAG.to_string())?;
info!("{RENAME_AVG_FUNCTION_MIGRATION_FLAG}, already_run: {avg_function_has_run}");

if !avg_function_has_run {
rewrite_items(&mut tx, None, |_tx, _cat, stmt| {
Box::pin(async { ast_rename_avg_function_0_66_0(stmt) })
})
.await?;
// Important: Mark the migration as being completed in the same transaction that we used
// to do the migration.
tx.mark_migration_has_run(RENAME_AVG_FUNCTION_MIGRATION_FLAG.to_string())?;
}

// Then, load up a temporary catalog with the rewritten items, and perform
// some transformations that require introspecting the catalog. These
// migrations are *weird*: they're rewriting the catalog while looking at
Expand Down Expand Up @@ -131,48 +110,6 @@ pub(crate) async fn migrate(
// AST migrations -- Basic AST -> AST transformations
// ****************************************************************************

fn ast_rename_avg_function_0_66_0(
stmt: &mut mz_sql::ast::Statement<Raw>,
) -> Result<(), anyhow::Error> {
struct AvgFunctionRenamer;
impl<'ast> VisitMut<'ast, Raw> for AvgFunctionRenamer {
fn visit_function_mut(&mut self, node: &'ast mut Function<Raw>) {
let components = &mut node.name.name_mut().0;
let len = components.len();
if len < 2 {
return;
}
let Some(last_two) = components.get_mut(len - 2..) else {
return;
};

// Rewrite the the function to be "avg_internal_v1".
if last_two[0].as_str() == "pg_catalog" && last_two[1].as_str() == "avg" {
last_two[0] = Ident::new("mz_catalog");
last_two[1] = Ident::new("avg_internal_v1");
}

// No one should have an object with this function as a dependency, but we check for it
// here just to be safe.
if last_two[0].as_str() == "mz_internal" && last_two[1].as_str() == "mz_avg_promotion" {
last_two[1] = Ident::new("mz_avg_promotion_internal_v1");
}

// Continue to recurse.
self.visit_function_args_mut(&mut node.args);
if let Some(v) = &mut node.filter {
self.visit_expr_mut(&mut *v);
}
if let Some(v) = &mut node.over {
self.visit_window_spec_mut(&mut *v);
}
}
}

AvgFunctionRenamer.visit_statement_mut(stmt);
Ok(())
}

// ****************************************************************************
// Semantic migrations -- Weird migrations that require access to the catalog
// ****************************************************************************
Expand Down

0 comments on commit 4460d47

Please sign in to comment.