From 0dfdd2174b3dbbdd5c53b76ade97768560a1df50 Mon Sep 17 00:00:00 2001 From: CVdV-au <69226052+CVdV-au@users.noreply.github.com> Date: Tue, 29 Oct 2024 09:14:35 +1100 Subject: [PATCH] Remove dead DBKVS code, fix Oracle hints (#7338) Oracle hints/query improved; dead DBKVS code removed --- .../keyvalue/dbkvs/impl/DbQueryFactory.java | 9 -- .../dbkvs/impl/oracle/OracleQueryFactory.java | 142 +++--------------- .../impl/postgres/PostgresQueryFactory.java | 67 --------- changelog/@unreleased/pr-7338.v2.yml | 5 + 4 files changed, 23 insertions(+), 200 deletions(-) create mode 100644 changelog/@unreleased/pr-7338.v2.yml diff --git a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/DbQueryFactory.java b/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/DbQueryFactory.java index bbe1dc9d12c..adf5683e501 100644 --- a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/DbQueryFactory.java +++ b/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/DbQueryFactory.java @@ -28,27 +28,18 @@ public interface DbQueryFactory { FullQuery getLatestRowsQuery(Iterable rows, long ts, ColumnSelection columns, boolean includeValue); - FullQuery getLatestRowsQuery( - Collection> rows, ColumnSelection columns, boolean includeValue); - FullQuery getAllRowQuery(byte[] row, long ts, ColumnSelection columns, boolean includeValue); FullQuery getAllRowsQuery(Iterable rows, long ts, ColumnSelection columns, boolean includeValue); - FullQuery getAllRowsQuery(Collection> rows, ColumnSelection columns, boolean includeValue); - FullQuery getLatestCellQuery(Cell cell, long ts, boolean includeValue); - FullQuery getLatestCellsQuery(Iterable cells, long ts, boolean includeValue); - FullQuery getLatestCellsQuery(Collection> cells, boolean includeValue); FullQuery getAllCellQuery(Cell cell, long ts, boolean includeValue); FullQuery getAllCellsQuery(Iterable cells, long ts, boolean includeValue); - FullQuery getAllCellsQuery(Collection> cells, boolean includeValue); - FullQuery getRangeQuery(RangeRequest range, long ts, int maxRows); boolean hasOverflowValues(); diff --git a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/oracle/OracleQueryFactory.java b/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/oracle/OracleQueryFactory.java index f6a07da7512..b6e5c7fbbd9 100644 --- a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/oracle/OracleQueryFactory.java +++ b/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/oracle/OracleQueryFactory.java @@ -45,10 +45,7 @@ public OracleQueryFactory(OracleDdlConfig config, String tableName, boolean hasO @Override public FullQuery getLatestRowQuery(byte[] row, long ts, ColumnSelection columns, boolean includeValue) { - String query = " /* GET_LATEST_ONE_ROW_INNER (" + tableName + ") */ " - + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " + String query = "SELECT" + " m.row_name, m.col_name, max(m.ts) as ts " + " FROM " + tableName + " m " + " WHERE m.row_name = ? " @@ -71,10 +68,8 @@ public FullQuery getLatestRowQuery(byte[] row, long ts, ColumnSelection columns, @Override public FullQuery getLatestRowsQuery(Iterable rows, long ts, ColumnSelection columns, boolean includeValue) { - String query = " /* GET_LATEST_ROWS_SINGLE_BOUND_INNER (" + tableName + ") */ " - + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " + String query = "SELECT" + + " /*+ USE_NL(t m) LEADING(t m) */ " + " m.row_name, m.col_name, max(m.ts) as ts " + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " + " WHERE m.row_name = t.row_name " @@ -95,39 +90,10 @@ public FullQuery getLatestRowsQuery(Iterable rows, long ts, ColumnSelect : fullQuery.withArg(rowsToOracleArray(columns.getSelectedColumns())); } - @Override - public FullQuery getLatestRowsQuery( - Collection> rows, ColumnSelection columns, boolean includeValue) { - String query = " /* GET_LATEST_ROWS_MANY_BOUNDS_INNER (" + tableName + ") */ " - + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " - + " m.row_name, m.col_name, max(m.ts) as ts " - + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " - + " WHERE m.row_name = t.row_name " - + " AND m.ts < t.max_ts " - + (columns.allColumnsSelected() - ? "" - : " AND EXISTS (" - + "SELECT" - + " /*+ NL_SJ */" - + " 1" - + " FROM TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE))" - + " WHERE row_name = m.col_name) ") - + " GROUP BY m.row_name, m.col_name"; - query = wrapQueryWithIncludeValue("GET_LATEST_ROWS_MANY_BOUNDS", query, includeValue); - FullQuery fullQuery = new FullQuery(query).withArg(rowsAndTimestampsToOracleArray(rows)); - return columns.allColumnsSelected() - ? fullQuery - : fullQuery.withArg(rowsToOracleArray(columns.getSelectedColumns())); - } - @Override public FullQuery getAllRowQuery(byte[] row, long ts, ColumnSelection columns, boolean includeValue) { String query = " /* GET_ALL_ONE_ROW (" + tableName + ") */ " - + " SELECT" - + " /*+ INDEX(m " + PrimaryKeyConstraintNames.get(tableName) + ") */ " - + " m.row_name, m.col_name, m.ts" + getValueSubselect("m", includeValue) + + " SELECT m.row_name, m.col_name, m.ts" + getValueSubselect("m", includeValue) + " FROM " + tableName + " m " + " WHERE m.row_name = ? " + " AND m.ts < ? " @@ -149,8 +115,7 @@ public FullQuery getAllRowQuery(byte[] row, long ts, ColumnSelection columns, bo public FullQuery getAllRowsQuery(Iterable rows, long ts, ColumnSelection columns, boolean includeValue) { String query = " /* GET_ALL_ROWS_SINGLE_BOUND (" + tableName + ") */ " + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " + + " /*+ USE_NL(t m) LEADING(t m) */" + " m.row_name, m.col_name, m.ts" + getValueSubselect("m", includeValue) + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " + " WHERE m.row_name = t.row_name " @@ -169,37 +134,9 @@ public FullQuery getAllRowsQuery(Iterable rows, long ts, ColumnSelection : fullQuery.withArg(rowsToOracleArray(columns.getSelectedColumns())); } - @Override - public FullQuery getAllRowsQuery( - Collection> rows, ColumnSelection columns, boolean includeValue) { - String query = " /* GET_ALL_ROWS_MANY_BOUNDS (" + tableName + ") */ " - + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " - + " m.row_name, m.col_name, m.ts" + getValueSubselect("m", includeValue) - + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " - + " WHERE m.row_name = t.row_name " - + " AND m.ts < t.max_ts " - + (columns.allColumnsSelected() - ? "" - : " AND EXISTS (" - + "SELECT" - + " /*+ NL_SJ */" - + " 1" - + " FROM TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE))" - + " WHERE row_name = m.col_name) "); - FullQuery fullQuery = new FullQuery(query).withArg(rowsAndTimestampsToOracleArray(rows)); - return columns.allColumnsSelected() - ? fullQuery - : fullQuery.withArg(rowsToOracleArray(columns.getSelectedColumns())); - } - @Override public FullQuery getLatestCellQuery(Cell cell, long ts, boolean includeValue) { - String query = " /* GET_LATEST_ONE_CELLS_INNER (" + tableName + ") */ " - + " SELECT " - + " /*+ INDEX(m " + PrimaryKeyConstraintNames.get(tableName) + ") */ " - + " m.row_name, m.col_name, max(m.ts) as ts " + String query = "SELECT m.row_name, m.col_name, max(m.ts) as ts" + " FROM " + tableName + " m " + " WHERE m.row_name = ? " + " AND m.col_name = ? " @@ -209,28 +146,10 @@ public FullQuery getLatestCellQuery(Cell cell, long ts, boolean includeValue) { return new FullQuery(query).withArgs(cell.getRowName(), cell.getColumnName(), ts); } - @Override - public FullQuery getLatestCellsQuery(Iterable cells, long ts, boolean includeValue) { - String query = " /* GET_LATEST_CELLS_SINGLE_BOUND_INNER (" + tableName + ") */ " - + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " - + " m.row_name, m.col_name, max(m.ts) as ts " - + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " - + " WHERE m.row_name = t.row_name " - + " AND m.col_name = t.col_name " - + " AND m.ts < ? " - + " GROUP BY m.row_name, m.col_name"; - query = wrapQueryWithIncludeValue("GET_LATEST_CELLS_SINGLE_BOUND", query, includeValue); - return new FullQuery(query).withArgs(cellsToOracleArray(cells), ts); - } - @Override public FullQuery getLatestCellsQuery(Collection> cells, boolean includeValue) { - String query = " /* GET_LATEST_CELLS_MANY_BOUNDS_INNER (" + tableName + ") */ " - + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " + String query = "SELECT" + + " /*+ USE_NL(t m) LEADING(t m) */ " + " m.row_name, m.col_name, max(m.ts) as ts " + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " + " WHERE m.row_name = t.row_name " @@ -245,7 +164,6 @@ public FullQuery getLatestCellsQuery(Collection> cells, bo public FullQuery getAllCellQuery(Cell cell, long ts, boolean includeValue) { String query = " /* GET_ALL_ONE_CELL (" + tableName + ") */ " + " SELECT" - + " /*+ INDEX(m " + PrimaryKeyConstraintNames.get(tableName) + ") */ " + " m.row_name, m.col_name, m.ts" + getValueSubselect("m", includeValue) + " FROM " + tableName + " m " + " WHERE m.row_name = ? " @@ -258,8 +176,7 @@ public FullQuery getAllCellQuery(Cell cell, long ts, boolean includeValue) { public FullQuery getAllCellsQuery(Iterable cells, long ts, boolean includeValue) { String query = " /* GET_ALL_CELLS_SINGLE_BOUND (" + tableName + ") */ " + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " + + " /*+ USE_NL(t m) LEADING(t m) */ " + " m.row_name, m.col_name, m.ts" + getValueSubselect("m", includeValue) + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " + " WHERE m.row_name = t.row_name " @@ -268,20 +185,6 @@ public FullQuery getAllCellsQuery(Iterable cells, long ts, boolean include return new FullQuery(query).withArgs(cellsToOracleArray(cells), ts); } - @Override - public FullQuery getAllCellsQuery(Collection> cells, boolean includeValue) { - String query = " /* GET_ALL_CELLS_MANY_BOUNDS (" + tableName + ") */ " - + " SELECT" - + " /*+ USE_NL(t m) LEADING(t m) CARDINALITY(t 1) CARDINALITY(m 10) INDEX(m " - + PrimaryKeyConstraintNames.get(tableName) + ") */ " - + " m.row_name, m.col_name, m.ts" + getValueSubselect("m", includeValue) - + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " - + " WHERE m.row_name = t.row_name " - + " AND m.col_name = t.col_name " - + " AND m.ts < t.max_ts "; - return new FullQuery(query).withArg(cellsAndTimestampsToOracleArray(cells)); - } - @Override public FullQuery getRangeQuery(RangeRequest range, long ts, int maxRows) { List bounds = new ArrayList<>(2); @@ -298,7 +201,7 @@ public FullQuery getRangeQuery(RangeRequest range, long ts, int maxRows) { } if (maxRows == 1) { String query = " /* GET_RANGE_ONE_ROW (" + tableName + ") */ " - + " SELECT /*+ INDEX(m " + PrimaryKeyConstraintNames.get(tableName) + ") */ " + + " SELECT " + (range.isReverse() ? "max" : "min") + "(m.row_name) as row_name " + " FROM " + tableName + " m " + (bounds.isEmpty() ? "" : " WHERE " + Joiner.on(" AND ").join(bounds)); @@ -307,7 +210,7 @@ public FullQuery getRangeQuery(RangeRequest range, long ts, int maxRows) { String query = " /* GET_RANGE_ROWS (" + tableName + ") */ " + " SELECT inner.row_name FROM " - + " ( SELECT /*+ INDEX(m " + PrimaryKeyConstraintNames.get(tableName) + ") */ " + + " ( SELECT" + " DISTINCT m.row_name " + " FROM " + tableName + " m " + (bounds.isEmpty() ? "" : " WHERE " + Joiner.on(" AND ").join(bounds)) @@ -373,15 +276,16 @@ protected FullQuery getRowsColumnRangeSubQuery( @Override protected FullQuery getRowsColumnRangeFullyLoadedRowsSubQuery( List rows, long ts, ColumnRangeSelection columnRangeSelection) { - String query = " /* GET_ROWS_COLUMN_RANGE_FULLY_LOADED_ROWS (" + tableName + ") */ " - + "SELECT * FROM ( SELECT m.row_name, m.col_name, max(m.ts) as ts" - + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " - + " WHERE m.row_name = t.row_name " - + " AND m.ts < ? " + String query = "SELECT" + + " /*+ USE_NL(t m) LEADING(t m) */" + + " m.row_name, m.col_name, max(m.ts) as ts" + + " FROM " + tableName + " m, TABLE(CAST(? AS " + structArrayPrefix() + "CELL_TS_TABLE)) t " + + " WHERE m.row_name = t.row_name " + + " AND m.ts < ? " + (columnRangeSelection.getStartCol().length > 0 ? " AND m.col_name >= ?" : "") + (columnRangeSelection.getEndCol().length > 0 ? " AND m.col_name < ?" : "") + " GROUP BY m.row_name, m.col_name" - + " ORDER BY m.row_name ASC, m.col_name ASC )"; + + " ORDER BY m.row_name ASC, m.col_name ASC"; String wrappedQuery = wrapQueryWithIncludeValue("GET_ROWS_COLUMN_RANGE_FULLY_LOADED_ROWS", query, true); FullQuery fullQuery = new FullQuery(wrappedQuery).withArgs(rowsToOracleArray(rows), ts); if (columnRangeSelection.getStartCol().length > 0) { @@ -436,16 +340,6 @@ private ArrayHandler cellsToOracleArray(Iterable cells) { structArrayPrefix() + "CELL_TS", "" + structArrayPrefix() + "CELL_TS_TABLE", oraRows); } - private ArrayHandler rowsAndTimestampsToOracleArray(Collection> rows) { - List oraRows = new ArrayList<>(rows.size()); - for (Map.Entry entry : rows) { - oraRows.add(new Object[] {entry.getKey(), null, entry.getValue()}); - } - return config.jdbcHandler() - .createStructArray( - structArrayPrefix() + "CELL_TS", "" + structArrayPrefix() + "CELL_TS_TABLE", oraRows); - } - private ArrayHandler cellsAndTimestampsToOracleArray(Collection> cells) { List oraRows = new ArrayList<>(cells.size()); for (Map.Entry entry : cells) { diff --git a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresQueryFactory.java b/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresQueryFactory.java index 09ea7a8a868..78f8bd1909d 100644 --- a/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresQueryFactory.java +++ b/atlasdb-dbkvs/src/main/java/com/palantir/atlasdb/keyvalue/dbkvs/impl/postgres/PostgresQueryFactory.java @@ -71,24 +71,6 @@ public FullQuery getLatestRowsQuery(Iterable rows, long ts, ColumnSelect return columns.allColumnsSelected() ? fullQuery : fullQuery.withArgs(columns.getSelectedColumns()); } - @Override - public FullQuery getLatestRowsQuery( - Collection> rows, ColumnSelection columns, boolean includeValue) { - String query = " /* GET_LATEST_ROWS_INNER (" + tableName + ") */ " - + " SELECT m.row_name, m.col_name, max(m.ts) as ts " - + " FROM " + prefixedTableName() + " m," - + " (VALUES " + groupOfNumParams(2, rows.size()) + ") t(row_name, ts) " - + " WHERE m.row_name = t.row_name " - + " AND m.ts < t.ts " - + (columns.allColumnsSelected() - ? "" - : " AND m.col_name IN " + numParams(Iterables.size(columns.getSelectedColumns()))) - + " GROUP BY m.row_name, m.col_name "; - query = wrapQueryWithIncludeValue("GET_LATEST_ROW", query, includeValue); - FullQuery fullQuery = addRowTsArgs(new FullQuery(query), rows); - return columns.allColumnsSelected() ? fullQuery : fullQuery.withArgs(columns.getSelectedColumns()); - } - @Override public FullQuery getAllRowQuery(byte[] row, long ts, ColumnSelection columns, boolean includeValue) { String query = " /* GET_ALL_ROW (" + tableName + ") */ " @@ -117,22 +99,6 @@ public FullQuery getAllRowsQuery(Iterable rows, long ts, ColumnSelection return columns.allColumnsSelected() ? fullQuery : fullQuery.withArgs(columns.getSelectedColumns()); } - @Override - public FullQuery getAllRowsQuery( - Collection> rows, ColumnSelection columns, boolean includeValue) { - String query = " /* GET_ALL_ROWS (" + tableName + ") */ " - + " SELECT m.row_name, m.col_name, m.ts" + (includeValue ? ", m.val " : " ") - + " FROM " + prefixedTableName() + " m," - + " (VALUES " + groupOfNumParams(2, rows.size()) + ") t(row_name, ts) " - + " WHERE m.row_name = t.row_name " - + " AND m.ts < t.ts " - + (columns.allColumnsSelected() - ? "" - : " AND m.col_name IN " + numParams(Iterables.size(columns.getSelectedColumns()))); - FullQuery fullQuery = addRowTsArgs(new FullQuery(query), rows); - return columns.allColumnsSelected() ? fullQuery : fullQuery.withArgs(columns.getSelectedColumns()); - } - @Override public FullQuery getLatestCellQuery(Cell cell, long ts, boolean includeValue) { String query = " /* GET_LATEST_CELL_INNER (" + tableName + ") */ " @@ -147,20 +113,6 @@ public FullQuery getLatestCellQuery(Cell cell, long ts, boolean includeValue) { return new FullQuery(query).withArgs(cell.getRowName(), cell.getColumnName(), ts); } - @Override - public FullQuery getLatestCellsQuery(Iterable cells, long ts, boolean includeValue) { - String query = " /* GET_LATEST_CELLS_INNER (" + tableName + ") */ " - + " SELECT m.row_name, m.col_name, max(m.ts) as ts " - + " FROM " + prefixedTableName() + " m," - + " (VALUES " + groupOfNumParams(2, Iterables.size(cells)) + ") t(row_name, col_name) " - + " WHERE m.row_name = t.row_name " - + " AND m.col_name = t.col_name " - + " AND m.ts < ? " - + " GROUP BY m.row_name, m.col_name "; - query = wrapQueryWithIncludeValue("GET_LATEST_CELLS", query, includeValue); - return addCellArgs(new FullQuery(query), cells).withArg(ts); - } - @Override public FullQuery getLatestCellsQuery(Collection> cells, boolean includeValue) { String query = " /* GET_LATEST_CELLS_INNER (" + tableName + ") */ " @@ -198,18 +150,6 @@ public FullQuery getAllCellsQuery(Iterable cells, long ts, boolean include return addCellArgs(new FullQuery(query), cells).withArg(ts); } - @Override - public FullQuery getAllCellsQuery(Collection> cells, boolean includeValue) { - String query = " /* GET_ALL_CELLS (" + tableName + ") */ " - + " SELECT m.row_name, m.col_name, m.ts" + (includeValue ? ", m.val " : " ") - + " FROM " + prefixedTableName() + " m," - + " (VALUES " + groupOfNumParams(3, Iterables.size(cells)) + ") t(row_name, col_name, ts) " - + " WHERE m.row_name = t.row_name " - + " AND m.col_name = t.col_name " - + " AND m.ts < t.ts "; - return addCellTsArgs(new FullQuery(query), cells); - } - @Override public FullQuery getRangeQuery(RangeRequest range, long ts, int maxRows) { List bounds = new ArrayList<>(2); @@ -261,13 +201,6 @@ private String wrapQueryWithIncludeValue(String wrappedName, String query, boole + " AND wrap.ts = i.ts "; } - private FullQuery addRowTsArgs(FullQuery fullQuery, Iterable> rows) { - for (Map.Entry entry : rows) { - fullQuery.withArgs(entry.getKey(), entry.getValue()); - } - return fullQuery; - } - private FullQuery addCellArgs(FullQuery fullQuery, Iterable cells) { for (Cell cell : cells) { fullQuery.withArgs(cell.getRowName(), cell.getColumnName()); diff --git a/changelog/@unreleased/pr-7338.v2.yml b/changelog/@unreleased/pr-7338.v2.yml new file mode 100644 index 00000000000..631c416c747 --- /dev/null +++ b/changelog/@unreleased/pr-7338.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Oracle hints/query improved; dead DBKVS code removed + links: + - https://github.com/palantir/atlasdb/pull/7338