From 7902b276add296569508ff08294d448ceeed97b4 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 19 Apr 2016 22:50:05 +0200 Subject: [PATCH] Support unneeded schema names in overviews queries Fixes #420 Keep table schema of overviews base tables and use it to support queries that use the schema name when not strictly needed. --- lib/cartodb/api/overviews_metadata_api.js | 7 +- lib/cartodb/utils/overviews_query_rewriter.js | 10 ++- test/acceptance/overviews_metadata.js | 1 + .../overviews_metadata_named_maps.js | 1 + .../mapconfig_overviews_adapter.js | 1 + test/integration/overviews-metadata-api.js | 1 + test/unit/cartodb/overviews_query_rewriter.js | 72 +++++++++++++++++++ 7 files changed, 91 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/api/overviews_metadata_api.js b/lib/cartodb/api/overviews_metadata_api.js index 738753f0..302a08f1 100644 --- a/lib/cartodb/api/overviews_metadata_api.js +++ b/lib/cartodb/api/overviews_metadata_api.js @@ -22,7 +22,10 @@ function prepareSql(sql) { } OverviewsMetadataApi.prototype.getOverviewsMetadata = function (username, sql, callback) { - var query = 'SELECT * FROM CDB_Overviews(CDB_QueryTablesText($windshaft$' + prepareSql(sql) + '$windshaft$))'; + // FIXME: Currently using internal function parsed_table + // CDB_Overviews should provide the schema information directly. + var query = 'SELECT *, _cdb_schema_name(base_table)' + + ' FROM CDB_Overviews(CDB_QueryTablesText($windshaft$' + prepareSql(sql) + '$windshaft$))'; this.pgQueryRunner.run(username, query, function handleOverviewsRows(err, rows) { if (err){ callback(err); @@ -31,11 +34,13 @@ OverviewsMetadataApi.prototype.getOverviewsMetadata = function (username, sql, c var metadata = {}; rows.forEach(function(row) { var table = row.base_table; + var schema = row._cdb_schema_name; var table_metadata = metadata[table]; if ( !table_metadata ) { table_metadata = metadata[table] = {}; } table_metadata[row.z] = { table: row.overview_table }; + table_metadata.schema = schema; }); return callback(null, metadata); }); diff --git a/lib/cartodb/utils/overviews_query_rewriter.js b/lib/cartodb/utils/overviews_query_rewriter.js index 2b253908..7f720062 100644 --- a/lib/cartodb/utils/overviews_query_rewriter.js +++ b/lib/cartodb/utils/overviews_query_rewriter.js @@ -22,7 +22,7 @@ function overviews_view_for_table(table, overviews_metadata, indent) { indent = indent || ' '; for (var z in overviews_metadata) { - if (overviews_metadata.hasOwnProperty(z)) { + if (overviews_metadata.hasOwnProperty(z) && z !== 'schema') { sorted_overviews.push([z, overviews_metadata[z].table]); } } @@ -127,8 +127,16 @@ function overviews_query(query, overviews, zoom_level_expression) { if (overviews.hasOwnProperty(table)) { var table_overviews = overviews[table]; var table_view = overviews_view_name(table); + var schema = table_overviews.schema; replacement = "(\n" + overviews_view_for_table(table, table_overviews) + "\n ) AS " + table_view; replaced_query = replace_table_in_query(replaced_query, table, replacement); + var parsed_table = TableNameParser.parse(table); + if (!parsed_table.schema && schema) { + // replace also the qualified table name, if the table wasn't qualified + parsed_table.schema = schema; + table = TableNameParser.table_identifier(parsed_table); + replaced_query = replace_table_in_query(replaced_query, table, replacement); + } } } if ( replaced_query !== query ) { diff --git a/test/acceptance/overviews_metadata.js b/test/acceptance/overviews_metadata.js index 0fa99bf9..81b90331 100644 --- a/test/acceptance/overviews_metadata.js +++ b/test/acceptance/overviews_metadata.js @@ -90,6 +90,7 @@ describe('overviews metadata', function() { var expected_data = { overviews: { test_table_overviews: { + schema: 'public', 1: { table: '_vovw_1_test_table_overviews' }, 2: { table: '_vovw_2_test_table_overviews' } } diff --git a/test/acceptance/overviews_metadata_named_maps.js b/test/acceptance/overviews_metadata_named_maps.js index 38326986..ff9a0389 100644 --- a/test/acceptance/overviews_metadata_named_maps.js +++ b/test/acceptance/overviews_metadata_named_maps.js @@ -136,6 +136,7 @@ describe('overviews metadata for named maps', function() { var expected_data = { overviews: { test_table_overviews: { + schema: 'public', 1: { table: '_vovw_1_test_table_overviews' }, 2: { table: '_vovw_2_test_table_overviews' } } diff --git a/test/integration/mapconfig_overviews_adapter.js b/test/integration/mapconfig_overviews_adapter.js index 079575a7..ba9e67e5 100644 --- a/test/integration/mapconfig_overviews_adapter.js +++ b/test/integration/mapconfig_overviews_adapter.js @@ -75,6 +75,7 @@ describe('MapConfigOverviewsAdapter', function() { var expected_data = { overviews: { test_table_overviews: { + schema: 'public', 1: { table: '_vovw_1_test_table_overviews' }, 2: { table: '_vovw_2_test_table_overviews' } } diff --git a/test/integration/overviews-metadata-api.js b/test/integration/overviews-metadata-api.js index f1182a6c..a5961750 100644 --- a/test/integration/overviews-metadata-api.js +++ b/test/integration/overviews-metadata-api.js @@ -40,6 +40,7 @@ describe('OverviewsMetadataApi', function() { assert.deepEqual(result, { 'test_table_overviews': { + schema: 'public', 1: { table: '_vovw_1_test_table_overviews' }, 2: { table: '_vovw_2_test_table_overviews' } } diff --git a/test/unit/cartodb/overviews_query_rewriter.js b/test/unit/cartodb/overviews_query_rewriter.js index f79e82c7..77b6881d 100644 --- a/test/unit/cartodb/overviews_query_rewriter.js +++ b/test/unit/cartodb/overviews_query_rewriter.js @@ -202,6 +202,78 @@ describe('Overviews query rewriter', function() { done(); }); + it('uses schema name from overviews', function(done){ + var sql = "SELECT * FROM public.table1"; + var data = { + overviews: { + 'table1': { + schema: 'public', + 2: { table: 'table1_ov2' } + } + } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data); + var expected_sql = "\ + WITH\ + _vovw_scale AS ( SELECT ZoomLevel() AS _vovw_z )\ + SELECT * FROM (\ + SELECT * FROM table1_ov2, _vovw_scale WHERE _vovw_z <= 2\ + UNION ALL\ + SELECT * FROM table1, _vovw_scale WHERE _vovw_z > 2\ + ) AS _vovw_table1\ + "; + assertSameSql(overviews_sql, expected_sql); + done(); + }); + + it('ignores schema name from overviews if not necessary', function(done){ + var sql = "SELECT * FROM table1"; + var data = { + overviews: { + 'table1': { + schema: 'public', + 2: { table: 'table1_ov2' } + } + } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data); + var expected_sql = "\ + WITH\ + _vovw_scale AS ( SELECT ZoomLevel() AS _vovw_z )\ + SELECT * FROM (\ + SELECT * FROM table1_ov2, _vovw_scale WHERE _vovw_z <= 2\ + UNION ALL\ + SELECT * FROM table1, _vovw_scale WHERE _vovw_z > 2\ + ) AS _vovw_table1\ + "; + assertSameSql(overviews_sql, expected_sql); + done(); + }); + + it('uses redundant schema information', function(done){ + var sql = "SELECT * FROM public.table1"; + var data = { + overviews: { + 'public.table1': { + schema: 'public', + 2: { table: 'table1_ov2' } + } + } + }; + var overviews_sql = overviewsQueryRewriter.query(sql, data); + var expected_sql = "\ + WITH\ + _vovw_scale AS ( SELECT ZoomLevel() AS _vovw_z )\ + SELECT * FROM (\ + SELECT * FROM public.table1_ov2, _vovw_scale WHERE _vovw_z <= 2\ + UNION ALL\ + SELECT * FROM public.table1, _vovw_scale WHERE _vovw_z > 2\ + ) AS _vovw_table1\ + "; + assertSameSql(overviews_sql, expected_sql); + done(); + }); + it('generates query for a table that needs quoting with explicit schema', function(done){ var sql = "SELECT * FROM public.\"table 1\""; var data = {