Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpSplitCodec.java (1)

52-63: ⚠️ Potential issue | 🟠 Major

Bounds-check the split ordinal before indexing the enum.

SplitType.values()[...] will throw on bad or future ordinals, which makes corrupted payloads fail with an opaque exception.

Proposed fix

-            ClpSplit.SplitType type = ClpSplit.SplitType.values()[in.readInt()];
+            int ordinal = in.readInt();
+            ClpSplit.SplitType[] values = ClpSplit.SplitType.values();
+            if (ordinal < 0 || ordinal >= values.length) {
+                throw new IllegalArgumentException("Unknown ClpSplit.SplitType ordinal: " + ordinal);
+            }
+            ClpSplit.SplitType type = values[ordinal];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpSplitCodec.java`
around lines 52 - 63, In ClpSplitCodec.deserialize, guard against invalid
ordinals before indexing ClpSplit.SplitType.values(): read the int ordinal into
a local variable, check it is within 0 (inclusive) and
ClpSplit.SplitType.values().length (exclusive), and if not throw a descriptive
IOException (or return a clear failure) instead of directly indexing; then map
the validated ordinal to ClpSplit.SplitType and continue building the ClpSplit
(refer to method deserialize and enum ClpSplit.SplitType and the ClpSplit
constructor).

presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java (1)

59-69: ⚠️ Potential issue | 🟠 Major

Guard the resolved type before constructing the handle.

TypeManager#getType(...) can return null; passing that through here would create an invalid ClpColumnHandle and shift the failure to a later call site.

Proposed fix

-            Type type = typeManager.getType(TypeSignature.parseTypeSignature(typeSignature));
-            return new ClpColumnHandle(columnName, originalColumnName, type);
+            Type type = typeManager.getType(TypeSignature.parseTypeSignature(typeSignature));
+            if (type == null) {
+                throw new IllegalArgumentException("Unknown CLP column type: " + typeSignature);
+            }
+            return new ClpColumnHandle(columnName, originalColumnName, type);

Read-only verification

#!/bin/bash
set -euo pipefail
sed -n '1,220p' presto-common/src/main/java/com/facebook/presto/common/type/TypeManager.java
rg -n -C 2 '\bgetType\s*\(' \
  presto-common/src/main/java/com/facebook/presto/common/type/TypeManager.java \
  presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java`
around lines 59 - 69, In ClpColumnHandleCodec.deserialize, guard against
TypeManager#getType returning null before constructing a ClpColumnHandle: after
resolving Type type = typeManager.getType(...), check if type == null and if so
throw a clear IOException (or other appropriate checked exception declared by
deserialize) with a message that includes the parsed typeSignature; otherwise
proceed to return new ClpColumnHandle(...). This ensures deserialize validates
the resolved type and fails fast instead of creating an invalid ClpColumnHandle.

presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java (1)

73-74: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when the type signature cannot be resolved.

At Line 73, typeManager.getType(...) can return null; Line 74 then builds a ClpColumnHandle with an invalid type. Please validate and throw an explicit exception with context (columnName, typeSignature) before constructing the handle.

Proposed fix

             String columnName = in.readUTF();
             String originalColumnName = in.readUTF();
             String typeSignature = in.readUTF();
-            Type type = typeManager.getType(TypeSignature.parseTypeSignature(typeSignature));
+            Type type = typeManager.getType(TypeSignature.parseTypeSignature(typeSignature));
+            if (type == null) {
+                throw new IllegalArgumentException("Unknown CLP column type: " + typeSignature + " for column: " + columnName);
+            }
             return new ClpColumnHandle(columnName, originalColumnName, type);

#!/bin/bash
set -euo pipefail

# Verify TypeManager#getType contract and ClpColumnHandle nullability expectations.
fd -i 'TypeManager.java' --exec sed -n '1,220p' {}
fd -i 'ClpColumnHandle.java' --exec sed -n '1,220p' {}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java`
around lines 73 - 74, Type resolution may return null so before constructing the
ClpColumnHandle you must validate the resolved Type and throw a clear exception
including columnName and typeSignature; update the code around Type type =
typeManager.getType(TypeSignature.parseTypeSignature(typeSignature)) to check if
type == null and throw an IllegalArgumentException (or PrestoException) with a
message like "Unable to resolve type for column <columnName>: <typeSignature>"
instead of returning a ClpColumnHandle with a null type; ensure you reference
TypeManager.getType, TypeSignature.parseTypeSignature, ClpColumnHandle,
columnName and typeSignature in the message for debugging.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java (1)

81-83: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when TypeManager cannot resolve the type signature.

On Line 81, if typeManager.getType(...) returns null, Line 82 creates an invalid ClpColumnHandle that fails later and less clearly.

Proposed fix

-            Type type = typeManager.getType(TypeSignature.parseTypeSignature(typeSignature));
+            Type type = typeManager.getType(TypeSignature.parseTypeSignature(typeSignature));
+            if (type == null) {
+                throw new IOException("Unknown CLP column type signature: " + typeSignature);
+            }
             return new ClpColumnHandle(columnName, originalColumnName, type);

#!/bin/bash
set -euo pipefail

# Verify TypeManager contract and current codec behaviour from source.
fd -i "TypeManager.java" --exec sed -n '1,220p' {}
fd -i "ClpColumnHandleCodec.java" --exec sed -n '1,220p' {}
fd -i "ClpColumnHandle.java" --exec sed -n '1,140p' {}

# Find other call sites guarding/null-checking getType results for precedent.
rg -n -C2 'getType\(TypeSignature\.parseTypeSignature' --iglob '*.java'

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java`
around lines 81 - 83, The deserialization code in ClpColumnHandleCodec calls
TypeManager.getType(TypeSignature.parseTypeSignature(typeSignature)) and can
return null, producing an invalid ClpColumnHandle; update the code in
ClpColumnHandleCodec (the block that calls TypeManager.getType /
TypeSignature.parseTypeSignature and constructs new ClpColumnHandle) to check
the returned Type for null and immediately throw an IllegalArgumentException (or
PrestoException if preferred) with a clear message containing the typeSignature
and columnName; this makes the failure fail-fast and avoids constructing an
invalid ClpColumnHandle.

presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpSplitCodec.java (1)

71-71: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate split-type ordinal before indexing enum values.

On Line 71, direct enum indexing can throw ArrayIndexOutOfBoundsException for malformed or drifted payloads; validate bounds and throw a clear deserialization error.

Proposed fix

-            ClpSplit.SplitType type = ClpSplit.SplitType.values()[in.readInt()];
+            int ordinal = in.readInt();
+            ClpSplit.SplitType[] values = ClpSplit.SplitType.values();
+            if (ordinal < 0 || ordinal >= values.length) {
+                throw new IOException("Unknown ClpSplit.SplitType ordinal: " + ordinal);
+            }
+            ClpSplit.SplitType type = values[ordinal];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpSplitCodec.java`
at line 71, Validate the ordinal read from the stream before indexing into the
enum: read the int into a local (e.g., "int ordinal = in.readInt()"), check that
ordinal >= 0 && ordinal < ClpSplit.SplitType.values().length, and if out of
bounds throw a clear deserialization exception (e.g., IOException or a
Presto-friendly exception) that includes the invalid ordinal; only then assign
ClpSplit.SplitType type = ClpSplit.SplitType.values()[ordinal]. This protects
ClpSplit.SplitType lookup from ArrayIndexOutOfBoundsException for malformed or
drifted payloads.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java (1)

82-83: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard unknown type signatures before constructing ClpColumnHandle.

typeManager.getType(...) can return null; on Line 83 this creates an invalid handle and defers failure to later paths. Fail fast with an explicit exception when type resolution fails.

Proposed fix

-            Type type = typeManager.getType(TypeSignature.parseTypeSignature(typeSignature));
-            return new ClpColumnHandle(columnName, originalColumnName, type);
+            TypeSignature parsedTypeSignature = TypeSignature.parseTypeSignature(typeSignature);
+            Type type = typeManager.getType(parsedTypeSignature);
+            if (type == null) {
+                throw new IllegalArgumentException("Unknown CLP column type signature: " + typeSignature +
+                        " for column: " + columnName);
+            }
+            return new ClpColumnHandle(columnName, originalColumnName, type);

#!/bin/bash
set -euo pipefail

# Verify TypeManager contract (null possible) and current codec behaviour.
sed -n '1,60p' presto-common/src/main/java/com/facebook/presto/common/type/TypeManager.java
sed -n '70,95p' presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java

# Check whether tests already cover unknown type-signature rejection.
rg -n --iglob '*TestClpConnectorCodecProvider.java' 'unknown|invalid|type signature|null.*type|deserialize' presto-clp/src/test/java

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@presto-clp/src/main/java/com/facebook/presto/plugin/clp/codec/ClpColumnHandleCodec.java`
around lines 82 - 83, The code currently calls
typeManager.getType(TypeSignature.parseTypeSignature(typeSignature)) and
immediately constructs a ClpColumnHandle, but getType can return null; modify
the ClpColumnHandleCodec decoding path so after resolving Type type =
typeManager.getType(...) you explicitly check for null and throw a clear
IllegalArgumentException (or PrestoException if preferred) including the
unresolved typeSignature and columnName, instead of returning a handle with a
null type; ensure the check is placed before the ClpColumnHandle constructor so
callers fail fast.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes