From 3c9ccbfe614c4f9fd16dac74e9cc444df62a4b92 Mon Sep 17 00:00:00 2001
From: CVdV-au <69226052+CVdV-au@users.noreply.github.com>
Date: Sat, 2 Nov 2024 01:34:09 +1100
Subject: [PATCH] Add explicit hints (#7418)

Add explicit NO_INDEX_FFS where Oracle seems sometimes to employ.
---
 .../dbkvs/impl/oracle/OracleQueryFactory.java  | 18 +++++++++---------
 changelog/@unreleased/pr-7418.v2.yml           |  7 +++++++
 2 files changed, 16 insertions(+), 9 deletions(-)
 create mode 100644 changelog/@unreleased/pr-7418.v2.yml

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 b6e5c7fbbd9..53fbc32eb43 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
@@ -69,7 +69,7 @@ public FullQuery getLatestRowQuery(byte[] row, long ts, ColumnSelection columns,
     @Override
     public FullQuery getLatestRowsQuery(Iterable<byte[]> rows, long ts, ColumnSelection columns, boolean includeValue) {
         String query = "SELECT"
-                + "   /*+ USE_NL(t m) LEADING(t m) */ "
+                + "   /*+ USE_NL(t m) LEADING(t m) NO_INDEX_FFS(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 "
@@ -115,7 +115,7 @@ public FullQuery getAllRowQuery(byte[] row, long ts, ColumnSelection columns, bo
     public FullQuery getAllRowsQuery(Iterable<byte[]> rows, long ts, ColumnSelection columns, boolean includeValue) {
         String query = " /* GET_ALL_ROWS_SINGLE_BOUND (" + tableName + ") */ "
                 + " SELECT"
-                + "   /*+ USE_NL(t m) LEADING(t m) */"
+                + "   /*+ USE_NL(t m) LEADING(t m) NO_INDEX_FFS(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 "
@@ -149,7 +149,7 @@ public FullQuery getLatestCellQuery(Cell cell, long ts, boolean includeValue) {
     @Override
     public FullQuery getLatestCellsQuery(Collection<Map.Entry<Cell, Long>> cells, boolean includeValue) {
         String query = "SELECT"
-                + "   /*+ USE_NL(t m) LEADING(t m) */ "
+                + "   /*+ USE_NL(t m) LEADING(t m) NO_INDEX_FFS(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 "
@@ -176,7 +176,7 @@ public FullQuery getAllCellQuery(Cell cell, long ts, boolean includeValue) {
     public FullQuery getAllCellsQuery(Iterable<Cell> cells, long ts, boolean includeValue) {
         String query = " /* GET_ALL_CELLS_SINGLE_BOUND (" + tableName + ") */ "
                 + " SELECT"
-                + "   /*+ USE_NL(t m) LEADING(t m) */ "
+                + "   /*+ USE_NL(t m) LEADING(t m) NO_INDEX_FFS(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 "
@@ -277,7 +277,7 @@ protected FullQuery getRowsColumnRangeSubQuery(
     protected FullQuery getRowsColumnRangeFullyLoadedRowsSubQuery(
             List<byte[]> rows, long ts, ColumnRangeSelection columnRangeSelection) {
         String query = "SELECT"
-                + " /*+ USE_NL(t m) LEADING(t m) */"
+                + " /*+ USE_NL(t m) LEADING(t m) NO_INDEX_FFS(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 "
@@ -297,15 +297,15 @@ protected FullQuery getRowsColumnRangeFullyLoadedRowsSubQuery(
         return fullQuery;
     }
 
-    private String wrapQueryWithIncludeValue(String wrappedName, String query, boolean includeValue) {
+    private String wrapQueryWithIncludeValue(String name, String query, boolean includeValue) {
         if (!includeValue) {
-            return query;
+            return "/* " + name + " (" + tableName + ") */ " + query;
         }
-        return " /* " + wrappedName + " (" + tableName + ") */ "
+        return "/* " + name + "_VALUE (" + tableName + ") */ "
                 + " SELECT"
                 + "   /*+ USE_NL(i wrap) LEADING(i wrap) NO_MERGE(i) NO_PUSH_PRED(i)"
                 + "       INDEX(wrap " + PrimaryKeyConstraintNames.get(tableName) + ") */ "
-                + "   wrap.row_name, wrap.col_name, wrap.ts" + getValueSubselect("wrap", includeValue)
+                + "   wrap.row_name, wrap.col_name, wrap.ts" + getValueSubselect("wrap", true)
                 + " FROM " + tableName + " wrap, ( " + query + " ) i "
                 + " WHERE wrap.row_name = i.row_name "
                 + "   AND wrap.col_name = i.col_name "
diff --git a/changelog/@unreleased/pr-7418.v2.yml b/changelog/@unreleased/pr-7418.v2.yml
new file mode 100644
index 00000000000..3848d63f6b6
--- /dev/null
+++ b/changelog/@unreleased/pr-7418.v2.yml
@@ -0,0 +1,7 @@
+type: fix
+fix:
+  description: |-
+    Add explicit hints, have seen Oracle incorrectly using FAST_FULL_SCAN on very large tables.
+    Possibly unexpected side effect from https://github.com/palantir/atlasdb/pull/7338 - even though the cardinality hints removed were misleading, they may have made Oracle behave.
+  links:
+  - https://github.com/palantir/atlasdb/pull/7418