Prevent caching INSERT statements (#3212)

This disables adding `INSERT` statements to the plan cache. The main
reason is related to potentially generating very large
`QueryPhysicalPlan` objects due to the way we capture individual
`INSERT` values and generate potentially massive `QueryPlanConstraint`
to verify their type (unnecessarily), in addition to other overheads
such as generating massive amount of redundant `Type` objects and `RCV`s
if the insert is touching a very wide table with hundreds of rows.

This resolves #3210

Author: hatyo

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/AstNormalizer.java

@@ -311,9 +311,21 @@ public Object visitDdlStatement(@Nonnull RelationalParser.DdlStatementContext ct
     }
 
     @Override
-    public Object visitDmlStatement(@Nonnull RelationalParser.DmlStatementContext ctx) {
-        queryCachingFlags.add(Result.QueryCachingFlags.IS_DML_STATEMENT);
-        return visitChildren(ctx);
+    public Object visitInsertStatement(final RelationalParser.InsertStatementContext ctx) {
+        queryCachingFlags.add(Result.QueryCachingFlags.IS_INSERT_STATEMENT);
+        return super.visitInsertStatement(ctx);
+    }
+
+    @Override
+    public Object visitUpdateStatement(final RelationalParser.UpdateStatementContext ctx) {
+        queryCachingFlags.add(Result.QueryCachingFlags.IS_UPDATE_STATEMENT);
+        return super.visitUpdateStatement(ctx);
+    }
+
+    @Override
+    public Object visitDeleteStatement(final RelationalParser.DeleteStatementContext ctx) {
+        queryCachingFlags.add(Result.QueryCachingFlags.IS_DELETE_STATEMENT);
+        return super.visitDeleteStatement(ctx);
     }
 
     @Override
@@ -600,7 +612,9 @@ public static class Result {
          */
         public enum QueryCachingFlags {
             IS_DDL_STATEMENT,
-            IS_DML_STATEMENT,
+            IS_UPDATE_STATEMENT,
+            IS_DELETE_STATEMENT,
+            IS_INSERT_STATEMENT,
             IS_DQL_STATEMENT,
             IS_UTILITY_STATEMENT,
             IS_ADMIN_STATEMENT,

fdb-relational-core/src/main/java/com/apple/foundationdb/relational/recordlayer/query/PlanGenerator.java

@@ -434,7 +434,11 @@ private static IndexFetchMethod toRecLayerIndexFetchMethod(@Nullable Options.Ind
      */
     private static boolean shouldNotCache(@Nonnull final Set<AstNormalizer.Result.QueryCachingFlags> queryCachingFlags) {
         return queryCachingFlags.contains(AstNormalizer.Result.QueryCachingFlags.WITH_NO_CACHE_OPTION) ||
-                queryCachingFlags.contains(AstNormalizer.Result.QueryCachingFlags.IS_DDL_STATEMENT);
+                queryCachingFlags.contains(AstNormalizer.Result.QueryCachingFlags.IS_DDL_STATEMENT) ||
+                // avoid caching INSERT statements since they could result in extremely large plans leading to potential
+                // OOM when too many of them are stored in the plan cache.
+                queryCachingFlags.contains(AstNormalizer.Result.QueryCachingFlags.IS_INSERT_STATEMENT);
+
     }
 
     /**

fdb-relational-core/src/test/java/com/apple/foundationdb/relational/recordlayer/query/AstNormalizerTests.java

@@ -607,41 +607,74 @@ void parseDqlStatementWithLogQuerySetLogQueryFlag() throws Exception {
     }
 
     @Test
-    void parseDmlStatementWithDryRunSetDryRunFalse() throws Exception {
+    void parseUpdateStatementWithoutDryRunSetsDryRunOptionsToFalse() throws Exception {
         validate(List.of("update A set A2 = 52 where A1 > 2"),
                 PreparedParams.empty(),
-                "update \"A\" set \"A2\" = ? where \"A1\" > ? ", // note: this is irrelevant as the query will be recompiled anyway.
+                "update \"A\" set \"A2\" = ? where \"A1\" > ? ",
                 List.of(Map.of(constantId(5), 52, constantId(9), 2)),
                 null,
                 -1,
-                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_DML_STATEMENT),
+                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_UPDATE_STATEMENT),
                 Map.of(Options.Name.DRY_RUN, false));
     }
 
     @Test
-    void parseDmlStatementWithDryRunSetDryRunTrue() throws Exception {
+    void parseUpdateStatementWithDryRunOptionSetsDryRunToTrue() throws Exception {
         validate(List.of("update A set A2 = 52 where A1 > 2 OPTIONS(DRY RUN)"),
                 PreparedParams.empty(),
-                "update \"A\" set \"A2\" = ? where \"A1\" > ? ", // note: this is irrelevant as the query will be recompiled anyway.
+                "update \"A\" set \"A2\" = ? where \"A1\" > ? ",
                 List.of(Map.of(constantId(5), 52, constantId(9), 2)),
                 null,
                 -1,
-                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_DML_STATEMENT),
+                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_UPDATE_STATEMENT),
                 Map.of(Options.Name.DRY_RUN, true));
     }
 
     @Test
-    void parseDmlStatementWithMultipleQueryOptions() throws Exception {
+    void parseUpdateStatementWithMultipleQueryOptions() throws Exception {
         validate(List.of("update A set A2 = 52 where A1 > 2 OPTIONS(DRY RUN, nocache, log query)"),
                 PreparedParams.empty(),
-                "update \"A\" set \"A2\" = ? where \"A1\" > ? ", // note: this is irrelevant as the query will be recompiled anyway.
+                "update \"A\" set \"A2\" = ? where \"A1\" > ? ",
                 List.of(Map.of(constantId(5), 52, constantId(9), 2)),
                 null,
                 -1,
-                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_DML_STATEMENT, AstNormalizer.Result.QueryCachingFlags.WITH_NO_CACHE_OPTION),
+                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_UPDATE_STATEMENT, AstNormalizer.Result.QueryCachingFlags.WITH_NO_CACHE_OPTION),
                 Map.of(Options.Name.DRY_RUN, true, Options.Name.LOG_QUERY, true));
     }
 
+    @Test
+    void parseUpdateStatementSetsQueryFlagCorrectly() throws Exception {
+        validate(List.of("update A set A2 = 52 where A1 > 2"),
+                PreparedParams.empty(),
+                "update \"A\" set \"A2\" = ? where \"A1\" > ? ",
+                List.of(Map.of(constantId(5), 52, constantId(9), 2)),
+                null,
+                -1,
+                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_UPDATE_STATEMENT));
+    }
+
+    @Test
+    void parseInsertStatementSetsQueryFlagCorrectly() throws Exception {
+        validate(List.of("insert into A values (42, 'foo')"),
+                PreparedParams.empty(),
+                "insert into \"A\" values ( ? , ? ) ",
+                List.of(Map.of(constantId(5), 42, constantId(7), "foo")),
+                null,
+                -1,
+                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_INSERT_STATEMENT));
+    }
+
+    @Test
+    void parseDeleteStatementSetsQueryFlagCorrectly() throws Exception {
+        validate(List.of("delete from A where x = 42"),
+                PreparedParams.empty(),
+                "delete from \"A\" where \"X\" = ? ",
+                List.of(Map.of(constantId(6), 42)),
+                null,
+                -1,
+                EnumSet.of(AstNormalizer.Result.QueryCachingFlags.IS_DELETE_STATEMENT));
+    }
+
     @Test
     void queryHashWorksWithPreparedParameters() throws Exception {
         validate(List.of(