From ebc480f09b62c7f1c00f1b5abba53ab0fa9160c3 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Thu, 20 Jul 2023 21:51:38 -0700 Subject: [PATCH 01/14] WIP - add ability to copy to existing mbtiles files --- martin-mbtiles/src/errors.rs | 6 ++ martin-mbtiles/src/mbtiles.rs | 53 ++++++++++- martin-mbtiles/src/tile_copier.rs | 150 +++++++++++++++++++----------- 3 files changed, 153 insertions(+), 56 deletions(-) diff --git a/martin-mbtiles/src/errors.rs b/martin-mbtiles/src/errors.rs index f058656d8..a9b071686 100644 --- a/martin-mbtiles/src/errors.rs +++ b/martin-mbtiles/src/errors.rs @@ -28,6 +28,12 @@ pub enum MbtError { #[error("The destination file {0} is non-empty")] NonEmptyTargetFile(PathBuf), + + #[error("The file {0} is does not have the required uniqueness constraint")] + NoUniquenessConstraint(String), + + #[error("Unexpected duplicate values found when copying")] + DuplicateValues(), } pub type MbtResult = Result; diff --git a/martin-mbtiles/src/mbtiles.rs b/martin-mbtiles/src/mbtiles.rs index 73079ec86..78b8675f8 100644 --- a/martin-mbtiles/src/mbtiles.rs +++ b/martin-mbtiles/src/mbtiles.rs @@ -2,6 +2,7 @@ extern crate core; +use std::collections::HashSet; use std::ffi::OsStr; use std::fmt::Display; use std::path::Path; @@ -11,7 +12,7 @@ use futures::TryStreamExt; use log::{debug, info, warn}; use martin_tile_utils::{Format, TileInfo}; use serde_json::{Value as JSONValue, Value}; -use sqlx::{query, SqliteExecutor}; +use sqlx::{query, Row, SqliteExecutor}; use tilejson::{tilejson, Bounds, Center, TileJSON}; use crate::errors::{MbtError, MbtResult}; @@ -26,7 +27,7 @@ pub struct Metadata { pub json: Option, } -#[derive(Debug, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub enum MbtType { TileTables, DeDuplicated, @@ -288,6 +289,54 @@ impl Mbtiles { Err(MbtError::InvalidDataFormat(self.filepath.clone())) } } + + pub async fn check_for_uniqueness_constraint( + &self, + conn: &mut T, + mbt_type: Option, + ) -> MbtResult<()> + where + for<'e> &'e mut T: SqliteExecutor<'e>, + { + let mbt_type = if let Some(mbt_type) = mbt_type { + mbt_type + } else { + self.detect_type(conn).await? + }; + + let table_name = match mbt_type { + MbtType::TileTables => "tiles", + MbtType::DeDuplicated => "map", + }; + + let mut unique_idx_cols = HashSet::new(); + let query_string = format!( + "SELECT DISTINCT ii.name as column_name + FROM + pragma_index_list('{table_name}') AS il, + pragma_index_info(il.name) AS ii + WHERE il.[unique] = 1;" + ); + let mut rows = query(&query_string).fetch(conn); + + while let Some(row) = rows.try_next().await? { + unique_idx_cols.insert(row.get("column_name")); + } + + if !unique_idx_cols + .symmetric_difference(&HashSet::from([ + "zoom_level".to_string(), + "tile_column".to_string(), + "tile_row".to_string(), + ])) + .collect::>() + .is_empty() + { + return Err(MbtError::NoUniquenessConstraint(self.filepath.clone())); + } + + Ok(()) + } } #[cfg(test)] diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 675395186..4019454a0 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -4,7 +4,7 @@ use std::collections::HashSet; use std::path::PathBuf; #[cfg(feature = "cli")] -use clap::{builder::ValueParser, error::ErrorKind, Args}; +use clap::{builder::ValueParser, error::ErrorKind, Args, ValueEnum}; use sqlx::sqlite::{SqliteArguments, SqliteConnectOptions}; use sqlx::{query, query_with, Arguments, Connection, Row, SqliteConnection}; @@ -13,6 +13,14 @@ use crate::mbtiles::MbtType; use crate::mbtiles::MbtType::TileTables; use crate::{MbtError, Mbtiles}; +#[derive(ValueEnum, PartialEq, Eq, Default, Debug, Clone)] +enum CopyDuplicateMode { + #[default] + Override, + Ignore, + Abort, +} + #[derive(Clone, Default, PartialEq, Eq, Debug)] #[cfg_attr(feature = "cli", derive(Args))] pub struct TileCopierOptions { @@ -21,8 +29,12 @@ pub struct TileCopierOptions { /// MBTiles file to write to dst_file: PathBuf, /// Force the output file to be in a simple MBTiles format with a `tiles` table + /// #[cfg_attr(feature = "cli", arg(long))] force_simple: bool, + /// Specify copying behaviour when tiles with duplicate (zoom_level, tile_column, tile_row) values are found + #[cfg_attr(feature = "cli", arg(long, value_enum, default_value_t = CopyDuplicateMode::Override))] + on_duplicate: CopyDuplicateMode, /// Minimum zoom level to copy #[cfg_attr(feature = "cli", arg(long, conflicts_with("zoom_levels")))] min_zoom: Option, @@ -72,6 +84,7 @@ impl clap::builder::TypedValueParser for HashSetValueParser { #[derive(Clone, Debug)] struct TileCopier { src_mbtiles: Mbtiles, + dst_mbtiles: Mbtiles, options: TileCopierOptions, } @@ -82,6 +95,7 @@ impl TileCopierOptions { dst_file: dst_filepath, zoom_levels: HashSet::new(), force_simple: false, + on_duplicate: CopyDuplicateMode::Override, min_zoom: None, max_zoom: None, diff_with_file: None, @@ -118,24 +132,27 @@ impl TileCopier { pub fn new(options: TileCopierOptions) -> MbtResult { Ok(TileCopier { src_mbtiles: Mbtiles::new(&options.src_file)?, + dst_mbtiles: Mbtiles::new(&options.dst_file)?, options, }) } + //TODO: handle case where source is simple and deduplicated pub async fn run(self) -> MbtResult { - let src_type = open_and_detect_type(&self.src_mbtiles).await?; - let force_simple = self.options.force_simple && src_type != MbtType::TileTables; + let mut mbtiles_type = open_and_detect_type(&self.src_mbtiles).await?; + let force_simple = self.options.force_simple && mbtiles_type != MbtType::TileTables; let opt = SqliteConnectOptions::new() .create_if_missing(true) .filename(&self.options.dst_file); let mut conn = SqliteConnection::connect_with(&opt).await?; - if query("SELECT 1 FROM sqlite_schema LIMIT 1") + let is_empty = query("SELECT 1 FROM sqlite_schema LIMIT 1") .fetch_optional(&mut conn) .await? - .is_some() - { + .is_none(); + + if !is_empty && self.options.diff_with_file.is_some() { return Err(MbtError::NonEmptyTargetFile(self.options.dst_file)); } @@ -147,42 +164,64 @@ impl TileCopier { .execute(&mut conn) .await?; - if force_simple { - for statement in &["CREATE TABLE metadata (name text, value text);", - "CREATE TABLE tiles (zoom_level integer, tile_column integer, tile_row integer, tile_data blob);", - "CREATE UNIQUE INDEX name on metadata (name);", - "CREATE UNIQUE INDEX tile_index on tiles (zoom_level, tile_column, tile_row);"] { - query(statement).execute(&mut conn).await?; - } + if is_empty { + if force_simple { + for statement in &["CREATE TABLE metadata (name text, value text);", + "CREATE TABLE tiles (zoom_level integer, tile_column integer, tile_row integer, tile_data blob);", + "CREATE UNIQUE INDEX name on metadata (name);", + "CREATE UNIQUE INDEX tile_index on tiles (zoom_level, tile_column, tile_row);"] { + query(statement).execute(&mut conn).await?; + } + } else { + // DB objects must be created in a specific order: tables, views, triggers, indexes. + + for row in query( + "SELECT sql + FROM sourceDb.sqlite_schema + WHERE tbl_name IN ('metadata', 'tiles', 'map', 'images') + AND type IN ('table', 'view', 'trigger', 'index') + ORDER BY CASE WHEN type = 'table' THEN 1 + WHEN type = 'view' THEN 2 + WHEN type = 'trigger' THEN 3 + WHEN type = 'index' THEN 4 + ELSE 5 END", + ) + .fetch_all(&mut conn) + .await? + { + query(row.get(0)).execute(&mut conn).await?; + } + }; + + query("INSERT INTO metadata SELECT * FROM sourceDb.metadata") + .execute(&mut conn) + .await?; } else { - // DB objects must be created in a specific order: tables, views, triggers, indexes. - - for row in query( - "SELECT sql - FROM sourceDb.sqlite_schema - WHERE tbl_name IN ('metadata', 'tiles', 'map', 'images') - AND type IN ('table', 'view', 'trigger', 'index') - ORDER BY CASE WHEN type = 'table' THEN 1 - WHEN type = 'view' THEN 2 - WHEN type = 'trigger' THEN 3 - WHEN type = 'index' THEN 4 - ELSE 5 END", - ) - .fetch_all(&mut conn) - .await? + mbtiles_type = open_and_detect_type(&self.dst_mbtiles).await?; + self.dst_mbtiles + .check_for_uniqueness_constraint(&mut conn, Some(mbtiles_type.clone())) + .await?; + + if self.options.on_duplicate == CopyDuplicateMode::Abort + && query( + "SELECT zoom_level, tile_column, tile_row + FROM tiles + INTERSECT + SELECT zoom_level, tile_column, tile_row + FROM sourceDb.tiles", + ) + .fetch_optional(&mut conn) + .await? + .is_some() { - query(row.get(0)).execute(&mut conn).await?; + return Err(MbtError::DuplicateValues()); } - }; - - query("INSERT INTO metadata SELECT * FROM sourceDb.metadata") - .execute(&mut conn) - .await?; + } if force_simple { self.copy_tile_tables(&mut conn).await? } else { - match src_type { + match mbtiles_type { MbtType::TileTables => self.copy_tile_tables(&mut conn).await?, MbtType::DeDuplicated => self.copy_deduplicated(&mut conn).await?, } @@ -223,25 +262,41 @@ impl TileCopier { self.run_query_with_options( conn, // Allows for adding clauses to query using "AND" - "INSERT INTO tiles SELECT * FROM sourceDb.tiles WHERE TRUE", + &format!( + "INSERT {} INTO tiles SELECT * FROM sourceDb.tiles WHERE TRUE", + match &self.options.on_duplicate { + CopyDuplicateMode::Override => "OR REPLACE", + CopyDuplicateMode::Ignore => "OR IGNORE", + _ => "", + } + ), ) .await } } async fn copy_deduplicated(&self, conn: &mut SqliteConnection) -> MbtResult<()> { - query("INSERT INTO map SELECT * FROM sourceDb.map") - .execute(&mut *conn) - .await?; + let on_duplicate_sql = match &self.options.on_duplicate { + CopyDuplicateMode::Override => "OR REPLACE", + CopyDuplicateMode::Ignore => "OR IGNORE", + _ => "", + }; + query(&format!( + "INSERT {on_duplicate_sql} INTO map SELECT * FROM sourceDb.map" + )) + .execute(&mut *conn) + .await?; self.run_query_with_options( conn, // Allows for adding clauses to query using "AND" - "INSERT INTO images + &format!( + "INSERT {on_duplicate_sql} INTO images SELECT images.tile_data, images.tile_id FROM sourceDb.images JOIN sourceDb.map ON images.tile_id = map.tile_id - WHERE TRUE", + WHERE TRUE" + ), ) .await } @@ -402,19 +457,6 @@ mod tests { verify_copy_all(src, dst).await; } - #[actix_rt::test] - async fn non_empty_target_file() { - let src = PathBuf::from("../tests/fixtures/files/world_cities.mbtiles"); - let dst = PathBuf::from("../tests/fixtures/files/json.mbtiles"); - assert!(matches!( - TileCopier::new(TileCopierOptions::new(src, dst)) - .unwrap() - .run() - .await, - Err(MbtError::NonEmptyTargetFile(_)) - )); - } - #[actix_rt::test] async fn copy_with_force_simple() { let src = PathBuf::from("../tests/fixtures/files/geography-class-jpg.mbtiles"); From 57d876fb97847716cab4ea0adef60ab12cbd4e90 Mon Sep 17 00:00:00 2001 From: rstanciu <55859815+upsicleclown@users.noreply.github.com> Date: Fri, 21 Jul 2023 06:52:55 -0700 Subject: [PATCH 02/14] Apply suggestions from code review Co-authored-by: Yuri Astrakhan --- martin-mbtiles/src/errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/martin-mbtiles/src/errors.rs b/martin-mbtiles/src/errors.rs index a9b071686..abd2ca628 100644 --- a/martin-mbtiles/src/errors.rs +++ b/martin-mbtiles/src/errors.rs @@ -29,10 +29,10 @@ pub enum MbtError { #[error("The destination file {0} is non-empty")] NonEmptyTargetFile(PathBuf), - #[error("The file {0} is does not have the required uniqueness constraint")] + #[error("The file {0} does not have the required uniqueness constraint")] NoUniquenessConstraint(String), - #[error("Unexpected duplicate values found when copying")] + #[error("Unexpected duplicate tiles found when copying")] DuplicateValues(), } From b4c99029cdaaf63d4eb891a2d04ab6ce64f555b6 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Fri, 21 Jul 2023 06:55:09 -0700 Subject: [PATCH 03/14] Address commants --- martin-mbtiles/src/tile_copier.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 4019454a0..0f62b8547 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -267,7 +267,7 @@ impl TileCopier { match &self.options.on_duplicate { CopyDuplicateMode::Override => "OR REPLACE", CopyDuplicateMode::Ignore => "OR IGNORE", - _ => "", + CopyDuplicateMode::Abort => "", } ), ) @@ -279,7 +279,7 @@ impl TileCopier { let on_duplicate_sql = match &self.options.on_duplicate { CopyDuplicateMode::Override => "OR REPLACE", CopyDuplicateMode::Ignore => "OR IGNORE", - _ => "", + CopyDuplicateMode::Abort => "", }; query(&format!( "INSERT {on_duplicate_sql} INTO map SELECT * FROM sourceDb.map" From 95b2c51a8eb7debd9136c821e322011e9b96135d Mon Sep 17 00:00:00 2001 From: rstanciu Date: Fri, 21 Jul 2023 07:38:00 -0700 Subject: [PATCH 04/14] Fix; Move cli trait under cli feature --- martin-mbtiles/src/tile_copier.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 0f62b8547..312ba0cba 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -13,7 +13,8 @@ use crate::mbtiles::MbtType; use crate::mbtiles::MbtType::TileTables; use crate::{MbtError, Mbtiles}; -#[derive(ValueEnum, PartialEq, Eq, Default, Debug, Clone)] +#[derive(PartialEq, Eq, Default, Debug, Clone)] +#[cfg_attr(feature = "cli", derive(ValueEnum))] enum CopyDuplicateMode { #[default] Override, From 6ed31959d930ae41cf7dcee5ca0196fde338acec Mon Sep 17 00:00:00 2001 From: rstanciu Date: Fri, 21 Jul 2023 07:47:02 -0700 Subject: [PATCH 05/14] Fix; Make CopyDuplicateMode public so to be used from lib --- martin-mbtiles/src/lib.rs | 4 +++- martin-mbtiles/src/tile_copier.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/martin-mbtiles/src/lib.rs b/martin-mbtiles/src/lib.rs index 198ffd592..59eaf2e3d 100644 --- a/martin-mbtiles/src/lib.rs +++ b/martin-mbtiles/src/lib.rs @@ -9,4 +9,6 @@ mod tile_copier; pub use errors::MbtError; pub use mbtiles::{Mbtiles, Metadata}; pub use mbtiles_pool::MbtilesPool; -pub use tile_copier::{apply_mbtiles_diff, copy_mbtiles_file, TileCopierOptions}; +pub use tile_copier::{ + apply_mbtiles_diff, copy_mbtiles_file, CopyDuplicateMode, TileCopierOptions, +}; diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 312ba0cba..26452f8f1 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -15,7 +15,7 @@ use crate::{MbtError, Mbtiles}; #[derive(PartialEq, Eq, Default, Debug, Clone)] #[cfg_attr(feature = "cli", derive(ValueEnum))] -enum CopyDuplicateMode { +pub enum CopyDuplicateMode { #[default] Override, Ignore, From 2d7d90639f54fc56b769a5b526ac5658fca47cc1 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Mon, 24 Jul 2023 16:17:36 -0700 Subject: [PATCH 06/14] Cleanup check for uniqueness constraint --- martin-mbtiles/src/mbtiles.rs | 25 ++++++++++++------------- martin-mbtiles/src/tile_copier.rs | 3 --- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/martin-mbtiles/src/mbtiles.rs b/martin-mbtiles/src/mbtiles.rs index 78b8675f8..fd29ff765 100644 --- a/martin-mbtiles/src/mbtiles.rs +++ b/martin-mbtiles/src/mbtiles.rs @@ -281,29 +281,28 @@ impl Mbtiles { where for<'e> &'e mut T: SqliteExecutor<'e>, { - if is_deduplicated_type(&mut *conn).await? { - Ok(MbtType::DeDuplicated) + let mbt_type = if is_deduplicated_type(&mut *conn).await? { + MbtType::DeDuplicated } else if is_tile_tables_type(&mut *conn).await? { - Ok(MbtType::TileTables) + MbtType::TileTables } else { - Err(MbtError::InvalidDataFormat(self.filepath.clone())) - } + return Err(MbtError::InvalidDataFormat(self.filepath.clone())); + }; + + self.check_for_uniqueness_constraint(&mut *conn, &mbt_type) + .await?; + + Ok(mbt_type) } - pub async fn check_for_uniqueness_constraint( + async fn check_for_uniqueness_constraint( &self, conn: &mut T, - mbt_type: Option, + mbt_type: &MbtType, ) -> MbtResult<()> where for<'e> &'e mut T: SqliteExecutor<'e>, { - let mbt_type = if let Some(mbt_type) = mbt_type { - mbt_type - } else { - self.detect_type(conn).await? - }; - let table_name = match mbt_type { MbtType::TileTables => "tiles", MbtType::DeDuplicated => "map", diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 26452f8f1..9ae24263c 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -199,9 +199,6 @@ impl TileCopier { .await?; } else { mbtiles_type = open_and_detect_type(&self.dst_mbtiles).await?; - self.dst_mbtiles - .check_for_uniqueness_constraint(&mut conn, Some(mbtiles_type.clone())) - .await?; if self.options.on_duplicate == CopyDuplicateMode::Abort && query( From f5f2d00053a2c64210476033efd9e268ee03ffcb Mon Sep 17 00:00:00 2001 From: rstanciu Date: Tue, 25 Jul 2023 13:57:21 -0700 Subject: [PATCH 07/14] Add tests --- martin-mbtiles/src/bin/main.rs | 65 ++++++++++++++++- martin-mbtiles/src/errors.rs | 2 +- martin-mbtiles/src/tile_copier.rs | 116 ++++++++++++++++++++++++++++-- 3 files changed, 177 insertions(+), 6 deletions(-) diff --git a/martin-mbtiles/src/bin/main.rs b/martin-mbtiles/src/bin/main.rs index e393bc15b..1da5241a3 100644 --- a/martin-mbtiles/src/bin/main.rs +++ b/martin-mbtiles/src/bin/main.rs @@ -93,7 +93,7 @@ mod tests { use clap::error::ErrorKind; use clap::Parser; - use martin_mbtiles::TileCopierOptions; + use martin_mbtiles::{CopyDuplicateMode, TileCopierOptions}; use crate::Args; use crate::Commands::{ApplyDiff, Copy, MetaGetValue}; @@ -245,6 +245,69 @@ mod tests { ); } + #[test] + fn test_copy_diff_with_override_copy_duplicate_mode() { + assert_eq!( + Args::parse_from([ + "mbtiles", + "copy", + "src_file", + "dst_file", + "--on-duplicate", + "override" + ]), + Args { + verbose: false, + command: Copy( + TileCopierOptions::new(PathBuf::from("src_file"), PathBuf::from("dst_file")) + .on_duplicate(CopyDuplicateMode::Override) + ) + } + ); + } + + #[test] + fn test_copy_diff_with_ignore_copy_duplicate_mode() { + assert_eq!( + Args::parse_from([ + "mbtiles", + "copy", + "src_file", + "dst_file", + "--on-duplicate", + "ignore" + ]), + Args { + verbose: false, + command: Copy( + TileCopierOptions::new(PathBuf::from("src_file"), PathBuf::from("dst_file")) + .on_duplicate(CopyDuplicateMode::Ignore) + ) + } + ); + } + + #[test] + fn test_copy_diff_with_abort_copy_duplicate_mode() { + assert_eq!( + Args::parse_from([ + "mbtiles", + "copy", + "src_file", + "dst_file", + "--on-duplicate", + "abort" + ]), + Args { + verbose: false, + command: Copy( + TileCopierOptions::new(PathBuf::from("src_file"), PathBuf::from("dst_file")) + .on_duplicate(CopyDuplicateMode::Abort) + ) + } + ); + } + #[test] fn test_meta_get_no_arguments() { assert_eq!( diff --git a/martin-mbtiles/src/errors.rs b/martin-mbtiles/src/errors.rs index abd2ca628..97c0e5a88 100644 --- a/martin-mbtiles/src/errors.rs +++ b/martin-mbtiles/src/errors.rs @@ -33,7 +33,7 @@ pub enum MbtError { NoUniquenessConstraint(String), #[error("Unexpected duplicate tiles found when copying")] - DuplicateValues(), + DuplicateValues, } pub type MbtResult = Result; diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 9ae24263c..3ce782f28 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -108,6 +108,11 @@ impl TileCopierOptions { self } + pub fn on_duplicate(mut self, on_duplicate: CopyDuplicateMode) -> Self { + self.on_duplicate = on_duplicate; + self + } + pub fn zoom_levels(mut self, zoom_levels: Vec) -> Self { self.zoom_levels.extend(zoom_levels); self @@ -157,15 +162,15 @@ impl TileCopier { return Err(MbtError::NonEmptyTargetFile(self.options.dst_file)); } - query("PRAGMA page_size = 512").execute(&mut conn).await?; - query("VACUUM").execute(&mut conn).await?; - query("ATTACH DATABASE ? AS sourceDb") .bind(self.src_mbtiles.filepath()) .execute(&mut conn) .await?; if is_empty { + query("PRAGMA page_size = 512").execute(&mut conn).await?; + query("VACUUM").execute(&mut conn).await?; + if force_simple { for statement in &["CREATE TABLE metadata (name text, value text);", "CREATE TABLE tiles (zoom_level integer, tile_column integer, tile_row integer, tile_data blob);", @@ -212,7 +217,7 @@ impl TileCopier { .await? .is_some() { - return Err(MbtError::DuplicateValues()); + return Err(MbtError::DuplicateValues); } } @@ -547,6 +552,109 @@ mod tests { .is_none()); } + #[actix_rt::test] + async fn copy_to_existing_abort_mode() { + let src = PathBuf::from("../tests/fixtures/files/world_cities_modified.mbtiles"); + let dst = PathBuf::from("../tests/fixtures/files/world_cities.mbtiles"); + + let copy_opts = + TileCopierOptions::new(src.clone(), dst.clone()).on_duplicate(CopyDuplicateMode::Abort); + + assert!(matches!( + copy_mbtiles_file(copy_opts).await.unwrap_err(), + MbtError::DuplicateValues + )); + } + + #[actix_rt::test] + async fn copy_to_existing_override_mode() { + let src_file = PathBuf::from("../tests/fixtures/files/world_cities_modified.mbtiles"); + + // Copy the dst file to an in-memory DB + let dst_file = PathBuf::from("../tests/fixtures/files/world_cities.mbtiles"); + let dst = + PathBuf::from("file:copy_to_existing_override_mode_mem_db?mode=memory&cache=shared"); + + let _dst_conn = copy_mbtiles_file(TileCopierOptions::new(dst_file.clone(), dst.clone())) + .await + .unwrap(); + + let mut dst_conn = copy_mbtiles_file(TileCopierOptions::new(src_file.clone(), dst.clone())) + .await + .unwrap(); + + // Verify the tiles in the destination file is a superset of the tiles in the source file + let _ = query("ATTACH DATABASE ? AS otherDb") + .bind(src_file.clone().to_str().unwrap()) + .execute(&mut dst_conn) + .await; + + assert!( + query("SELECT * FROM otherDb.tiles EXCEPT SELECT * FROM tiles;") + .fetch_optional(&mut dst_conn) + .await + .unwrap() + .is_none() + ); + } + + #[actix_rt::test] + async fn copy_to_existing_ignore_mode() { + let src_file = PathBuf::from("../tests/fixtures/files/world_cities_modified.mbtiles"); + + // Copy the dst file to an in-memory DB + let dst_file = PathBuf::from("../tests/fixtures/files/world_cities.mbtiles"); + let dst = + PathBuf::from("file:copy_to_existing_ignore_mode_mem_db?mode=memory&cache=shared"); + + let _dst_conn = copy_mbtiles_file(TileCopierOptions::new(dst_file.clone(), dst.clone())) + .await + .unwrap(); + + let mut dst_conn = copy_mbtiles_file( + TileCopierOptions::new(src_file.clone(), dst.clone()) + .on_duplicate(CopyDuplicateMode::Ignore), + ) + .await + .unwrap(); + + // Verify the tiles in the destination file are the same as those in the source file except for those with duplicate (zoom_level, tile_column, tile_row) + let _ = query("ATTACH DATABASE ? AS srcDb") + .bind(src_file.clone().to_str().unwrap()) + .execute(&mut dst_conn) + .await; + let _ = query("ATTACH DATABASE ? AS originalDb") + .bind(dst_file.clone().to_str().unwrap()) + .execute(&mut dst_conn) + .await; + + assert!( + query(" + SELECT * FROM + (SELECT * FROM + (SELECT t2.zoom_level, t2.tile_column, t2.tile_row, COALESCE(t1.tile_data, t2.tile_data) + FROM originalDb.tiles as t1 + RIGHT JOIN srcDb.tiles as t2 + ON t1.zoom_level=t2.zoom_level AND t1.tile_column=t2.tile_column AND t1.tile_row=t2.tile_row) + EXCEPT + SELECT * FROM tiles) + UNION + SELECT * FROM + (SELECT * FROM srcDb.tiles + EXCEPT + SELECT * FROM + (SELECT t2.zoom_level, t2.tile_column, t2.tile_row, COALESCE(t1.tile_data, t2.tile_data) + FROM srcDb.tiles as t1 + RIGHT JOIN tiles as t2 + ON t1.zoom_level=t2.zoom_level AND t1.tile_column=t2.tile_column AND t1.tile_row=t2.tile_row)) + ") + .fetch_optional(&mut dst_conn) + .await + .unwrap() + .is_none() + ); + } + #[actix_rt::test] async fn apply_diff_file() { // Copy the src file to an in-memory DB From c792494386ab5872771a5ae918b1c1b94f2dab1f Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 25 Jul 2023 19:06:22 -0400 Subject: [PATCH 08/14] Use sqlx macros more --- justfile | 2 +- ...a167b406de9961ac3cc69649c6152a6d7a9b7.json | 12 +++++ ...de564de47dde1d588f17e68ec58115ac73a39.json | 20 +++++++++ ...b7766f876ddb9a357a512ab3a37914bea003c.json | 12 +++++ ...7e779ecf324e1862945fbd18da4bf5baf565b.json | 12 +++++ ...ad480aaa224721cd9ddb4ac5859f71a57727e.json | 12 +++++ ...1f52ce710d8978e3b35b59b724fc5bee9f55c.json | 12 +++++ ...ad8c695c4c2350f294aefd210eccec603d905.json | 12 +++++ martin-mbtiles/src/tile_copier.rs | 44 ++++++++++--------- 9 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 martin-mbtiles/.sqlx/query-0a4540e8c33c71222a68ff5ecc1a167b406de9961ac3cc69649c6152a6d7a9b7.json create mode 100644 martin-mbtiles/.sqlx/query-176e99c6945b0789119d0d21a99de564de47dde1d588f17e68ec58115ac73a39.json create mode 100644 martin-mbtiles/.sqlx/query-3b2930e8d61f31ea1bf32efe340b7766f876ddb9a357a512ab3a37914bea003c.json create mode 100644 martin-mbtiles/.sqlx/query-a115609880b2c6ed3beeb5aaf8c7e779ecf324e1862945fbd18da4bf5baf565b.json create mode 100644 martin-mbtiles/.sqlx/query-b3aaef71d6a26404c3bebcc6ee8ad480aaa224721cd9ddb4ac5859f71a57727e.json create mode 100644 martin-mbtiles/.sqlx/query-e13e2e17d5bf56287bc0fd7c55a1f52ce710d8978e3b35b59b724fc5bee9f55c.json create mode 100644 martin-mbtiles/.sqlx/query-f547ff198e3bb604550a3f191e4ad8c695c4c2350f294aefd210eccec603d905.json diff --git a/justfile b/justfile index 9198c252e..46e003777 100644 --- a/justfile +++ b/justfile @@ -207,7 +207,7 @@ git-pre-push: stop start # Update sqlite database schema. prepare-sqlite: install-sqlx mkdir -p martin-mbtiles/.sqlx - cd martin-mbtiles && cargo sqlx prepare --database-url sqlite://$PWD/../tests/fixtures/files/world_cities.mbtiles + cd martin-mbtiles && cargo sqlx prepare --database-url sqlite://$PWD/../tests/fixtures/files/world_cities.mbtiles -- --lib --tests # Install SQLX cli if not already installed. [private] diff --git a/martin-mbtiles/.sqlx/query-0a4540e8c33c71222a68ff5ecc1a167b406de9961ac3cc69649c6152a6d7a9b7.json b/martin-mbtiles/.sqlx/query-0a4540e8c33c71222a68ff5ecc1a167b406de9961ac3cc69649c6152a6d7a9b7.json new file mode 100644 index 000000000..3f962a272 --- /dev/null +++ b/martin-mbtiles/.sqlx/query-0a4540e8c33c71222a68ff5ecc1a167b406de9961ac3cc69649c6152a6d7a9b7.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "VACUUM", + "describe": { + "columns": [], + "parameters": { + "Right": 0 + }, + "nullable": [] + }, + "hash": "0a4540e8c33c71222a68ff5ecc1a167b406de9961ac3cc69649c6152a6d7a9b7" +} diff --git a/martin-mbtiles/.sqlx/query-176e99c6945b0789119d0d21a99de564de47dde1d588f17e68ec58115ac73a39.json b/martin-mbtiles/.sqlx/query-176e99c6945b0789119d0d21a99de564de47dde1d588f17e68ec58115ac73a39.json new file mode 100644 index 000000000..25354e993 --- /dev/null +++ b/martin-mbtiles/.sqlx/query-176e99c6945b0789119d0d21a99de564de47dde1d588f17e68ec58115ac73a39.json @@ -0,0 +1,20 @@ +{ + "db_name": "SQLite", + "query": "SELECT 1 as has_rows FROM sqlite_schema LIMIT 1", + "describe": { + "columns": [ + { + "name": "has_rows", + "ordinal": 0, + "type_info": "Int" + } + ], + "parameters": { + "Right": 0 + }, + "nullable": [ + false + ] + }, + "hash": "176e99c6945b0789119d0d21a99de564de47dde1d588f17e68ec58115ac73a39" +} diff --git a/martin-mbtiles/.sqlx/query-3b2930e8d61f31ea1bf32efe340b7766f876ddb9a357a512ab3a37914bea003c.json b/martin-mbtiles/.sqlx/query-3b2930e8d61f31ea1bf32efe340b7766f876ddb9a357a512ab3a37914bea003c.json new file mode 100644 index 000000000..1a8fe4fd0 --- /dev/null +++ b/martin-mbtiles/.sqlx/query-3b2930e8d61f31ea1bf32efe340b7766f876ddb9a357a512ab3a37914bea003c.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "ATTACH DATABASE ? AS sourceDb", + "describe": { + "columns": [], + "parameters": { + "Right": 1 + }, + "nullable": [] + }, + "hash": "3b2930e8d61f31ea1bf32efe340b7766f876ddb9a357a512ab3a37914bea003c" +} diff --git a/martin-mbtiles/.sqlx/query-a115609880b2c6ed3beeb5aaf8c7e779ecf324e1862945fbd18da4bf5baf565b.json b/martin-mbtiles/.sqlx/query-a115609880b2c6ed3beeb5aaf8c7e779ecf324e1862945fbd18da4bf5baf565b.json new file mode 100644 index 000000000..fc0b3c0c2 --- /dev/null +++ b/martin-mbtiles/.sqlx/query-a115609880b2c6ed3beeb5aaf8c7e779ecf324e1862945fbd18da4bf5baf565b.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "ATTACH DATABASE ? AS newDb", + "describe": { + "columns": [], + "parameters": { + "Right": 1 + }, + "nullable": [] + }, + "hash": "a115609880b2c6ed3beeb5aaf8c7e779ecf324e1862945fbd18da4bf5baf565b" +} diff --git a/martin-mbtiles/.sqlx/query-b3aaef71d6a26404c3bebcc6ee8ad480aaa224721cd9ddb4ac5859f71a57727e.json b/martin-mbtiles/.sqlx/query-b3aaef71d6a26404c3bebcc6ee8ad480aaa224721cd9ddb4ac5859f71a57727e.json new file mode 100644 index 000000000..71fbbc367 --- /dev/null +++ b/martin-mbtiles/.sqlx/query-b3aaef71d6a26404c3bebcc6ee8ad480aaa224721cd9ddb4ac5859f71a57727e.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "ATTACH DATABASE ? AS otherDb", + "describe": { + "columns": [], + "parameters": { + "Right": 1 + }, + "nullable": [] + }, + "hash": "b3aaef71d6a26404c3bebcc6ee8ad480aaa224721cd9ddb4ac5859f71a57727e" +} diff --git a/martin-mbtiles/.sqlx/query-e13e2e17d5bf56287bc0fd7c55a1f52ce710d8978e3b35b59b724fc5bee9f55c.json b/martin-mbtiles/.sqlx/query-e13e2e17d5bf56287bc0fd7c55a1f52ce710d8978e3b35b59b724fc5bee9f55c.json new file mode 100644 index 000000000..5d8f76197 --- /dev/null +++ b/martin-mbtiles/.sqlx/query-e13e2e17d5bf56287bc0fd7c55a1f52ce710d8978e3b35b59b724fc5bee9f55c.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "ATTACH DATABASE ? AS diffDb", + "describe": { + "columns": [], + "parameters": { + "Right": 1 + }, + "nullable": [] + }, + "hash": "e13e2e17d5bf56287bc0fd7c55a1f52ce710d8978e3b35b59b724fc5bee9f55c" +} diff --git a/martin-mbtiles/.sqlx/query-f547ff198e3bb604550a3f191e4ad8c695c4c2350f294aefd210eccec603d905.json b/martin-mbtiles/.sqlx/query-f547ff198e3bb604550a3f191e4ad8c695c4c2350f294aefd210eccec603d905.json new file mode 100644 index 000000000..cdcdb555c --- /dev/null +++ b/martin-mbtiles/.sqlx/query-f547ff198e3bb604550a3f191e4ad8c695c4c2350f294aefd210eccec603d905.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "PRAGMA page_size = 512", + "describe": { + "columns": [], + "parameters": { + "Right": 0 + }, + "nullable": [] + }, + "hash": "f547ff198e3bb604550a3f191e4ad8c695c4c2350f294aefd210eccec603d905" +} diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 3ce782f28..5bdf272af 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -10,7 +10,7 @@ use sqlx::{query, query_with, Arguments, Connection, Row, SqliteConnection}; use crate::errors::MbtResult; use crate::mbtiles::MbtType; -use crate::mbtiles::MbtType::TileTables; +use crate::mbtiles::MbtType::{DeDuplicated, TileTables}; use crate::{MbtError, Mbtiles}; #[derive(PartialEq, Eq, Default, Debug, Clone)] @@ -146,14 +146,14 @@ impl TileCopier { //TODO: handle case where source is simple and deduplicated pub async fn run(self) -> MbtResult { let mut mbtiles_type = open_and_detect_type(&self.src_mbtiles).await?; - let force_simple = self.options.force_simple && mbtiles_type != MbtType::TileTables; + let force_simple = self.options.force_simple && mbtiles_type != TileTables; let opt = SqliteConnectOptions::new() .create_if_missing(true) .filename(&self.options.dst_file); let mut conn = SqliteConnection::connect_with(&opt).await?; - let is_empty = query("SELECT 1 FROM sqlite_schema LIMIT 1") + let is_empty = query!("SELECT 1 as has_rows FROM sqlite_schema LIMIT 1") .fetch_optional(&mut conn) .await? .is_none(); @@ -162,14 +162,14 @@ impl TileCopier { return Err(MbtError::NonEmptyTargetFile(self.options.dst_file)); } - query("ATTACH DATABASE ? AS sourceDb") - .bind(self.src_mbtiles.filepath()) + let path = self.src_mbtiles.filepath(); + query!("ATTACH DATABASE ? AS sourceDb", path) .execute(&mut conn) .await?; if is_empty { - query("PRAGMA page_size = 512").execute(&mut conn).await?; - query("VACUUM").execute(&mut conn).await?; + query!("PRAGMA page_size = 512").execute(&mut conn).await?; + query!("VACUUM").execute(&mut conn).await?; if force_simple { for statement in &["CREATE TABLE metadata (name text, value text);", @@ -225,8 +225,8 @@ impl TileCopier { self.copy_tile_tables(&mut conn).await? } else { match mbtiles_type { - MbtType::TileTables => self.copy_tile_tables(&mut conn).await?, - MbtType::DeDuplicated => self.copy_deduplicated(&mut conn).await?, + TileTables => self.copy_tile_tables(&mut conn).await?, + DeDuplicated => self.copy_deduplicated(&mut conn).await?, } } @@ -240,8 +240,8 @@ impl TileCopier { // because all types will be used in the same way open_and_detect_type(&diff_with_mbtiles).await?; - query("ATTACH DATABASE ? AS newDb") - .bind(diff_with_mbtiles.filepath()) + let path = diff_with_mbtiles.filepath(); + query!("ATTACH DATABASE ? AS newDb", path) .execute(&mut *conn) .await?; @@ -360,7 +360,7 @@ pub async fn apply_mbtiles_diff( let mut conn = SqliteConnection::connect_with(&opt).await?; let src_type = src_mbtiles.detect_type(&mut conn).await?; - if src_type != MbtType::TileTables { + if src_type != TileTables { return Err(MbtError::IncorrectDataFormat( src_mbtiles.filepath().to_string(), TileTables, @@ -370,8 +370,8 @@ pub async fn apply_mbtiles_diff( open_and_detect_type(&diff_mbtiles).await?; - query("ATTACH DATABASE ? AS diffDb") - .bind(diff_mbtiles.filepath()) + let path = diff_mbtiles.filepath(); + query!("ATTACH DATABASE ? AS diffDb", path) .execute(&mut conn) .await?; @@ -379,9 +379,13 @@ pub async fn apply_mbtiles_diff( " DELETE FROM tiles WHERE (zoom_level, tile_column, tile_row) IN - (SELECT zoom_level, tile_column, tile_row FROM diffDb.tiles WHERE tile_data ISNULL); + (SELECT zoom_level, tile_column, tile_row FROM diffDb.tiles WHERE tile_data ISNULL);", + ) + .execute(&mut conn) + .await?; - INSERT OR REPLACE INTO tiles (zoom_level, tile_column, tile_row, tile_data) + query( + "INSERT OR REPLACE INTO tiles (zoom_level, tile_column, tile_row, tile_data) SELECT * FROM diffDb.tiles WHERE tile_data NOTNULL;", ) .execute(&mut conn) @@ -670,11 +674,11 @@ mod tests { let mut src_conn = apply_mbtiles_diff(src, diff_file).await.unwrap(); // Verify the data is the same as the file the diff was generated from - let modified_file = PathBuf::from("../tests/fixtures/files/world_cities_modified.mbtiles"); - let _ = query("ATTACH DATABASE ? AS otherDb") - .bind(modified_file.clone().to_str().unwrap()) + let path = "../tests/fixtures/files/world_cities_modified.mbtiles"; + query!("ATTACH DATABASE ? AS otherDb", path) .execute(&mut src_conn) - .await; + .await + .unwrap(); assert!( query("SELECT * FROM tiles EXCEPT SELECT * FROM otherDb.tiles;") From 7aaebdd32fb9d7b8a0379cf88fb68cef3d9b47f5 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Tue, 25 Jul 2023 17:14:12 -0700 Subject: [PATCH 09/14] Handle copying from simple to deduplicated --- martin-mbtiles/src/errors.rs | 3 +++ martin-mbtiles/src/tile_copier.rs | 25 +++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/martin-mbtiles/src/errors.rs b/martin-mbtiles/src/errors.rs index 97c0e5a88..ff314118f 100644 --- a/martin-mbtiles/src/errors.rs +++ b/martin-mbtiles/src/errors.rs @@ -32,6 +32,9 @@ pub enum MbtError { #[error("The file {0} does not have the required uniqueness constraint")] NoUniquenessConstraint(String), + #[error("Could not copy MBTiles file: {reason}")] + UnsupportedCopyOperation { reason: String }, + #[error("Unexpected duplicate tiles found when copying")] DuplicateValues, } diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 5bdf272af..aa22d5972 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -143,7 +143,6 @@ impl TileCopier { }) } - //TODO: handle case where source is simple and deduplicated pub async fn run(self) -> MbtResult { let mut mbtiles_type = open_and_detect_type(&self.src_mbtiles).await?; let force_simple = self.options.force_simple && mbtiles_type != TileTables; @@ -203,7 +202,16 @@ impl TileCopier { .execute(&mut conn) .await?; } else { - mbtiles_type = open_and_detect_type(&self.dst_mbtiles).await?; + let dst_type = open_and_detect_type(&self.dst_mbtiles).await?; + + if mbtiles_type == TileTables && dst_type == DeDuplicated { + return Err(MbtError::UnsupportedCopyOperation{ reason: "\ + Attempted copying from a source file with simple format to a non-empty destination file with deduplicated format, which is not currently supported" + .to_string(), + }); + } + + mbtiles_type = dst_type; if self.options.on_duplicate == CopyDuplicateMode::Abort && query( @@ -556,6 +564,19 @@ mod tests { .is_none()); } + #[actix_rt::test] + async fn copy_from_simple_to_existing_deduplicated() { + let src = PathBuf::from("../tests/fixtures/files/world_cities_modified.mbtiles"); + let dst = PathBuf::from("../tests/fixtures/files/geography-class-jpg.mbtiles"); + + let copy_opts = TileCopierOptions::new(src.clone(), dst.clone()); + + assert!(matches!( + copy_mbtiles_file(copy_opts).await.unwrap_err(), + MbtError::UnsupportedCopyOperation { .. } + )); + } + #[actix_rt::test] async fn copy_to_existing_abort_mode() { let src = PathBuf::from("../tests/fixtures/files/world_cities_modified.mbtiles"); From 5d1dd2a04026f4f250c38ab6742cad9f57303261 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Wed, 26 Jul 2023 14:52:51 -0700 Subject: [PATCH 10/14] Fix copying in abort mode; fail fast and allow duplicate tile_data --- martin-mbtiles/src/tile_copier.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index aa22d5972..1aa6246f6 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -215,11 +215,10 @@ impl TileCopier { if self.options.on_duplicate == CopyDuplicateMode::Abort && query( - "SELECT zoom_level, tile_column, tile_row - FROM tiles - INTERSECT - SELECT zoom_level, tile_column, tile_row - FROM sourceDb.tiles", + "SELECT * FROM tiles t1 + JOIN sourceDb.tiles t2 + ON t1.zoom_level=t2.zoom_level AND t1.tile_column=t2.tile_column AND t1.tile_row=t2.tile_row AND t1.tile_data!=t2.tile_data + LIMIT 1", ) .fetch_optional(&mut conn) .await? @@ -277,8 +276,7 @@ impl TileCopier { "INSERT {} INTO tiles SELECT * FROM sourceDb.tiles WHERE TRUE", match &self.options.on_duplicate { CopyDuplicateMode::Override => "OR REPLACE", - CopyDuplicateMode::Ignore => "OR IGNORE", - CopyDuplicateMode::Abort => "", + CopyDuplicateMode::Ignore | CopyDuplicateMode::Abort => "OR IGNORE", } ), ) @@ -289,8 +287,7 @@ impl TileCopier { async fn copy_deduplicated(&self, conn: &mut SqliteConnection) -> MbtResult<()> { let on_duplicate_sql = match &self.options.on_duplicate { CopyDuplicateMode::Override => "OR REPLACE", - CopyDuplicateMode::Ignore => "OR IGNORE", - CopyDuplicateMode::Abort => "", + CopyDuplicateMode::Ignore | CopyDuplicateMode::Abort => "OR IGNORE", }; query(&format!( "INSERT {on_duplicate_sql} INTO map SELECT * FROM sourceDb.map" From 4aaa71d4bfc22ccbc6b57d530d04d32c2d27c433 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Wed, 26 Jul 2023 15:15:25 -0700 Subject: [PATCH 11/14] When copying deduplicated tiles, copy images table first and cleanup unused images rows --- martin-mbtiles/src/tile_copier.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 1aa6246f6..5e2cdcac3 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -289,8 +289,11 @@ impl TileCopier { CopyDuplicateMode::Override => "OR REPLACE", CopyDuplicateMode::Ignore | CopyDuplicateMode::Abort => "OR IGNORE", }; + query(&format!( - "INSERT {on_duplicate_sql} INTO map SELECT * FROM sourceDb.map" + "INSERT {on_duplicate_sql} INTO images + SELECT images.tile_data, images.tile_id + FROM sourceDb.images" )) .execute(&mut *conn) .await?; @@ -298,15 +301,15 @@ impl TileCopier { self.run_query_with_options( conn, // Allows for adding clauses to query using "AND" - &format!( - "INSERT {on_duplicate_sql} INTO images - SELECT images.tile_data, images.tile_id - FROM sourceDb.images JOIN sourceDb.map - ON images.tile_id = map.tile_id - WHERE TRUE" - ), + &format!("INSERT {on_duplicate_sql} INTO map SELECT * FROM sourceDb.map WHERE TRUE"), ) - .await + .await?; + + query("DELETE FROM images WHERE tile_id NOT IN (SELECT DISTINCT tile_id FROM map)") + .execute(&mut *conn) + .await?; + + Ok(()) } async fn run_query_with_options( From 031686c2df6e91b12a5a5ae1ad919853a9b9a6b6 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Wed, 26 Jul 2023 17:46:33 -0700 Subject: [PATCH 12/14] Fix tests --- martin-mbtiles/src/tile_copier.rs | 63 ++++++++++++++++--------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/martin-mbtiles/src/tile_copier.rs b/martin-mbtiles/src/tile_copier.rs index 5e2cdcac3..338ea44b3 100644 --- a/martin-mbtiles/src/tile_copier.rs +++ b/martin-mbtiles/src/tile_copier.rs @@ -609,10 +609,11 @@ mod tests { .unwrap(); // Verify the tiles in the destination file is a superset of the tiles in the source file - let _ = query("ATTACH DATABASE ? AS otherDb") + query("ATTACH DATABASE ? AS otherDb") .bind(src_file.clone().to_str().unwrap()) .execute(&mut dst_conn) - .await; + .await + .unwrap(); assert!( query("SELECT * FROM otherDb.tiles EXCEPT SELECT * FROM tiles;") @@ -644,40 +645,40 @@ mod tests { .unwrap(); // Verify the tiles in the destination file are the same as those in the source file except for those with duplicate (zoom_level, tile_column, tile_row) - let _ = query("ATTACH DATABASE ? AS srcDb") + query("ATTACH DATABASE ? AS srcDb") .bind(src_file.clone().to_str().unwrap()) .execute(&mut dst_conn) - .await; - let _ = query("ATTACH DATABASE ? AS originalDb") + .await + .unwrap(); + query("ATTACH DATABASE ? AS originalDb") .bind(dst_file.clone().to_str().unwrap()) .execute(&mut dst_conn) - .await; - - assert!( - query(" - SELECT * FROM - (SELECT * FROM - (SELECT t2.zoom_level, t2.tile_column, t2.tile_row, COALESCE(t1.tile_data, t2.tile_data) + .await + .unwrap(); + // Create a temporary table with all the tiles in the original database and + // all the tiles in the source database except for those that conflict with tiles in the original database + query("CREATE TEMP TABLE expected_tiles AS + SELECT COALESCE(t1.zoom_level, t2.zoom_level) as zoom_level, + COALESCE(t1.tile_column, t2.zoom_level) as tile_column, + COALESCE(t1.tile_row, t2.tile_row) as tile_row, + COALESCE(t1.tile_data, t2.tile_data) as tile_data FROM originalDb.tiles as t1 - RIGHT JOIN srcDb.tiles as t2 - ON t1.zoom_level=t2.zoom_level AND t1.tile_column=t2.tile_column AND t1.tile_row=t2.tile_row) - EXCEPT - SELECT * FROM tiles) - UNION - SELECT * FROM - (SELECT * FROM srcDb.tiles - EXCEPT - SELECT * FROM - (SELECT t2.zoom_level, t2.tile_column, t2.tile_row, COALESCE(t1.tile_data, t2.tile_data) - FROM srcDb.tiles as t1 - RIGHT JOIN tiles as t2 - ON t1.zoom_level=t2.zoom_level AND t1.tile_column=t2.tile_column AND t1.tile_row=t2.tile_row)) - ") - .fetch_optional(&mut dst_conn) - .await - .unwrap() - .is_none() - ); + FULL OUTER JOIN srcDb.tiles as t2 + ON t1.zoom_level=t2.zoom_level AND t1.tile_column=t2.tile_column AND t1.tile_row=t2.tile_row") + .execute(&mut dst_conn) + .await + .unwrap(); + + // Ensure all entries in expected_tiles are in tiles and vice versa + assert!(query( + "SELECT * FROM expected_tiles EXCEPT SELECT * FROM tiles + UNION + SELECT * FROM tiles EXCEPT SELECT * FROM expected_tiles" + ) + .fetch_optional(&mut dst_conn) + .await + .unwrap() + .is_none()); } #[actix_rt::test] From e96e2d64933362c40b5192702bf4c4859b62d56a Mon Sep 17 00:00:00 2001 From: rstanciu Date: Wed, 26 Jul 2023 18:17:34 -0700 Subject: [PATCH 13/14] Test all indexes on tiles for unique constraint --- martin-mbtiles/src/mbtiles.rs | 52 +++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/martin-mbtiles/src/mbtiles.rs b/martin-mbtiles/src/mbtiles.rs index fd29ff765..6a32bc49f 100644 --- a/martin-mbtiles/src/mbtiles.rs +++ b/martin-mbtiles/src/mbtiles.rs @@ -308,33 +308,37 @@ impl Mbtiles { MbtType::DeDuplicated => "map", }; - let mut unique_idx_cols = HashSet::new(); - let query_string = format!( - "SELECT DISTINCT ii.name as column_name - FROM - pragma_index_list('{table_name}') AS il, - pragma_index_info(il.name) AS ii - WHERE il.[unique] = 1;" - ); - let mut rows = query(&query_string).fetch(conn); - - while let Some(row) = rows.try_next().await? { - unique_idx_cols.insert(row.get("column_name")); - } + let index_query = + format!("SELECT name FROM pragma_index_list('{table_name}') WHERE [unique] = 1;"); + let indexes = query(&index_query).fetch_all(&mut *conn).await?; + + // Ensure there is some index on tiles that has a unique constraint on (zoom_level, tile_row, tile_column) + for index in indexes { + let mut unique_idx_cols = HashSet::new(); + let key_column_query = format!( + "SELECT DISTINCT name FROM pragma_index_info('{}')", + index.get::("name") + ); + let rows = query(&key_column_query).fetch_all(&mut *conn).await?; + + for row in rows { + unique_idx_cols.insert(row.get("name")); + } - if !unique_idx_cols - .symmetric_difference(&HashSet::from([ - "zoom_level".to_string(), - "tile_column".to_string(), - "tile_row".to_string(), - ])) - .collect::>() - .is_empty() - { - return Err(MbtError::NoUniquenessConstraint(self.filepath.clone())); + if unique_idx_cols + .symmetric_difference(&HashSet::from([ + "zoom_level".to_string(), + "tile_column".to_string(), + "tile_row".to_string(), + ])) + .collect::>() + .is_empty() + { + return Ok(()); + } } - Ok(()) + Err(MbtError::NoUniquenessConstraint(self.filepath.clone())) } } From 738ff3c878177e794bf10bf44aced132cafe3603 Mon Sep 17 00:00:00 2001 From: rstanciu Date: Thu, 27 Jul 2023 11:22:35 -0700 Subject: [PATCH 14/14] Address comments; fix uniqueness check --- martin-mbtiles/src/mbtiles.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/martin-mbtiles/src/mbtiles.rs b/martin-mbtiles/src/mbtiles.rs index 6a32bc49f..d3efe3b51 100644 --- a/martin-mbtiles/src/mbtiles.rs +++ b/martin-mbtiles/src/mbtiles.rs @@ -308,18 +308,18 @@ impl Mbtiles { MbtType::DeDuplicated => "map", }; - let index_query = - format!("SELECT name FROM pragma_index_list('{table_name}') WHERE [unique] = 1;"); - let indexes = query(&index_query).fetch_all(&mut *conn).await?; + let indexes = query("SELECT name FROM pragma_index_list(?) WHERE [unique] = 1") + .bind(table_name) + .fetch_all(&mut *conn) + .await?; // Ensure there is some index on tiles that has a unique constraint on (zoom_level, tile_row, tile_column) for index in indexes { let mut unique_idx_cols = HashSet::new(); - let key_column_query = format!( - "SELECT DISTINCT name FROM pragma_index_info('{}')", - index.get::("name") - ); - let rows = query(&key_column_query).fetch_all(&mut *conn).await?; + let rows = query("SELECT DISTINCT name FROM pragma_index_info(?)") + .bind(index.get::("name")) + .fetch_all(&mut *conn) + .await?; for row in rows { unique_idx_cols.insert(row.get("name"));