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.

Sign up for GitHub

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

Jump to bottom

fix ST_EstimatedExtent computation #103

Merged
merged 1 commit into from
Mar 18, 2024
Merged

fix ST_EstimatedExtent computation #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 43 additions & 26 deletions martin/src/pg/query_tables.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,6 @@ pub async fn table_to_query(
bounds_type: BoundsCalcType,
max_feature_count: Option<usize>,
) -> PgResult<(String, PgSqlInfo, TableInfo)> {
let schema = escape_identifier(&info.schema);
let table = escape_identifier(&info.table);
let geometry_column = escape_identifier(&info.geometry_column);
let srid = info.srid;

if info.bounds.is_none() {
Expand Down Expand Up @@ -200,6 +197,9 @@ pub async fn table_to_query(
let limit_clause = max_feature_count.map_or(String::new(), |v| format!("LIMIT {v}"));
let layer_id = escape_literal(info.layer_id.as_ref().unwrap_or(&id));
let clip_geom = info.clip_geom.unwrap_or(DEFAULT_CLIP_GEOM);
let schema = escape_identifier(&info.schema);
let table = escape_identifier(&info.table);
let geometry_column = escape_identifier(&info.geometry_column);
let query = format!(
r#"
SELECT
Expand Down Expand Up @@ -233,19 +233,28 @@ async fn calc_bounds(
table: &str,
geometry_column: &str,
srid: i32,
is_quick: bool,
mut is_quick: bool,
) -> PgResult<Option<Bounds>> {
let sql = if is_quick {
let schema = escape_literal(schema);
let table = escape_literal(table);
let geometry_column = escape_literal(geometry_column);
format!("SELECT ST_EstimatedExtent({schema}, {table}, {geometry_column}) as bounds")
} else {
let schema = escape_identifier(schema);
let table = escape_identifier(table);
let geometry_column = escape_identifier(geometry_column);
format!(
r#"
let schema = escape_identifier(schema);
let table = escape_identifier(table);

let cn = pool.get().await?;
loop {
let query = if is_quick {
// This method is faster but less accurate, and can fail in a number of cases (returns NULL)
cn.query_one(
"SELECT ST_Transform(ST_SetSRID(ST_EstimatedExtent($1, $2, $3)::geometry, $4), 4326) as bounds",
&[
&&schema[1..schema.len() - 1],
&&table[1..table.len() - 1],
&geometry_column,
&srid,
],
).await
} else {
let geometry_column = escape_identifier(geometry_column);
cn.query_one(
&format!(r#"
WITH real_bounds AS (SELECT ST_SetSRID(ST_Extent({geometry_column}), {srid}) AS rb FROM {schema}.{table})
SELECT ST_Transform(
CASE
Expand All @@ -256,16 +265,24 @@ SELECT ST_Transform(
4326
) AS bounds
FROM {schema}.{table};
"#
)
};
"#),
&[]
).await
};

Ok(pool
.get()
.await?
.query_one(&sql, &[])
.await
.map_err(|e| PostgresError(e, "querying table bounds"))?
.get::<_, Option<ewkb::Polygon>>("bounds")
.and_then(|p| polygon_to_bbox(&p)))
if let Some(bounds) = query
.map_err(|e| PostgresError(e, "querying table bounds"))?
.get::<_, Option<ewkb::Polygon>>("bounds")
{
return Ok(polygon_to_bbox(&bounds));
}
if is_quick {
// ST_EstimatedExtent failed probably because there is no index or statistics or if it's a view
// This can only happen once if we are in quick mode
is_quick = false;
warn!("ST_EstimatedExtent on {schema}.{table}.{geometry_column} failed, trying slower method to compute bounds");
Copy link
Owner

@sharkAndshark sharkAndshark Mar 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The user with many big and special naming table and a auto_bounds: quick would still get a slow result. Is this what they want? I'm not sure.
But it would be rare.

Copy link
Author

Choose a reason for hiding this comment

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

exactly, this is not a common case. My bigger concern is that we currently do not cancel a query after 5 seconds - something we may need to dig deeper into postgres_tokio crate

} else {
return Ok(None);
}
}
}
24 changes: 12 additions & 12 deletions tests/expected/configured/save_config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ postgres:
geometry_column: Geom
id_column: giD
bounds:
- -170.94984639004662
- -84.20025580733805
- 167.70892858284475
- 74.23573284753762
- -170.94985961914062
- -84.20025634765625
- 167.7089385986328
- 74.23573303222656
geometry_type: POINT
properties:
taBLe: text
Expand All @@ -37,10 +37,10 @@ postgres:
geometry_column: geom
id_column: feat_id
bounds:
- -166.87107126230424
- -53.44747249115674
- 168.14061220360549
- 84.22411861475385
- -166.87107849121094
- -53.44747543334961
- 168.140625
- 84.22412109375
extent: 9000
buffer: 3
clip_geom: false
Expand All @@ -54,10 +54,10 @@ postgres:
geometry_column: geom
id_column: big_feat_id
bounds:
- -174.89475564568033
- -77.2579745396886
- 174.72753224514435
- 73.80785950599903
- -174.89476013183594
- -77.25798034667969
- 174.7275390625
- 73.807861328125
extent: 9000
buffer: 3
clip_geom: false
Expand Down
8 changes: 4 additions & 4 deletions tests/expected/configured/tbl_comment_cfg.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@
}
],
"bounds": [
-170.94984639004662,
-84.20025580733805,
167.70892858284475,
74.23573284753762
-170.94985961914062,
-84.20025634765625,
167.7089385986328,
74.23573303222656
],
"description": "a description from comment on table",
"name": "MixPoints"
Expand Down