case test patches

[Swiddis]

Description

workaround for missed optimizations

Related Issues

related: opensearch-project/OpenSearch#22091 (not blocking)

Check List

• New functionality includes testing.
• New functionality has been documented.
• New functionality has javadoc added.
• New functionality has a user manual doc added.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit a435c53)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review String Concatenation Error Lines 52-53 and 58-60 have malformed string concatenation. The opening quote for the JSON string is on the previous line, but the concatenation operator starts the next line, creating invalid syntax. This will cause a compilation error. The concatenation operator should be at the end of line 51 (before the newline), not at the start of line 52. "{\"host\": \"0.0.0.2\", \"method\": \"GET\", \"url\":" + " \"/shuttle/missions/sts-73/mission-sts-73.html\", \"response\": \"500\"," + " \"bytes\": \"4085\"}");

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to a435c53

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Extract complex condition into helper method The condition combines two different feature flags with OR logic. Consider extracting this into a descriptive helper method to improve readability and maintainability, especially if this pattern is repeated elsewhere in the codebase. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLCaseFunctionIT.java [313] -if (isPushdownDisabled() || isAnalyticsParquetIndicesEnabled()) { +if (shouldSkipRangeQueryOptimization()) { Suggestion importance[1-10]: 5 __ Why: The suggestion to extract the condition isPushdownDisabled() || isAnalyticsParquetIndicesEnabled() into a helper method like shouldSkipRangeQueryOptimization() is valid and would improve code readability and maintainability. However, since this appears only twice in the PR diff and the suggestion doesn't address a critical issue, it receives a moderate score for code quality improvement.	Low

---

Previous suggestions

Suggestions up to commit 70787c6

Category	Suggestion	Impact
General	Clarify idempotency mechanism in comment The comment claims idempotency via PUT, but the method doesn't verify if documents already exist before inserting. Consider adding a check or handling potential conflicts to ensure true idempotent behavior, especially if the method is called multiple times during test setup. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLCaseFunctionIT.java [43] -// These documents use fixed IDs (7-10), so reinserting is idempotent via PUT +// These documents use fixed IDs (7-10). PUT operations will overwrite existing documents, +// ensuring idempotent behavior when reinserting test data Suggestion importance[1-10]: 4 __ Why: The suggestion improves comment clarity by better explaining how PUT operations achieve idempotency through overwriting. However, the original comment is already accurate - PUT with fixed IDs is inherently idempotent, so this is a minor documentation enhancement rather than a correctness issue.	Low

Suggestions up to commit 8af66f7

Category	Suggestion	Impact
General	Use AtomicBoolean for thread-safety Using volatile alone doesn't guarantee thread-safety for check-then-act operations. The synchronized method combined with volatile creates redundancy. Consider using AtomicBoolean instead for a cleaner, lock-free implementation. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLCaseFunctionIT.java [28] -private static volatile boolean dataAppended = false; +private static final AtomicBoolean dataAppended = new AtomicBoolean(false); Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that volatile with synchronized creates redundancy. Using AtomicBoolean would be a cleaner approach for this check-then-act pattern, improving code quality and maintainability.	Medium
	Use compareAndSet for atomic operation If switching to AtomicBoolean, use compareAndSet(false, true) to atomically check and set the flag. This eliminates the need for synchronization and prevents race conditions more efficiently. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLCaseFunctionIT.java [43-47] -private synchronized void appendDataForBadResponse() throws IOException { - if (dataAppended) { +private void appendDataForBadResponse() throws IOException { + if (!dataAppended.compareAndSet(false, true)) { return; } - dataAppended = true; Suggestion importance[1-10]: 7 __ Why: This suggestion builds on the previous one and correctly shows how to use AtomicBoolean.compareAndSet() for a lock-free implementation. However, it depends on the first suggestion being applied and provides only moderate improvement in this test context.	Medium

Suggestions up to commit 8af66f7

Category	Suggestion	Impact
Possible issue	Use AtomicBoolean for thread-safe flag Using volatile alone doesn't guarantee thread-safety for check-then-act operations. The pattern if (dataAppended) return; dataAppended = true; can still have race conditions where multiple threads pass the check before any sets the flag. Consider using AtomicBoolean with compareAndSet() for proper thread-safe initialization. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLCaseFunctionIT.java [28] -private static volatile boolean dataAppended = false; +private static final AtomicBoolean dataAppended = new AtomicBoolean(false); Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a thread-safety issue with the volatile boolean pattern. Using AtomicBoolean with compareAndSet() provides proper atomic check-and-set semantics, preventing race conditions in the double-checked locking pattern.	Medium
General	Replace synchronized with compareAndSet operation If using AtomicBoolean, replace the synchronized method with compareAndSet() to avoid redundant synchronization. This ensures only one thread performs the data append operation while maintaining thread-safety without method-level locking. integ-test/src/test/java/org/opensearch/sql/calcite/remote/CalcitePPLCaseFunctionIT.java [43-47] -private synchronized void appendDataForBadResponse() throws IOException { - if (dataAppended) { +private void appendDataForBadResponse() throws IOException { + if (!dataAppended.compareAndSet(false, true)) { return; } - dataAppended = true; Suggestion importance[1-10]: 7 __ Why: This suggestion is a logical follow-up to suggestion 1. If AtomicBoolean is adopted, using compareAndSet() eliminates the need for method-level synchronization, providing better performance while maintaining thread-safety. However, it depends on implementing suggestion 1 first.	Medium

[github-actions]

Persistent review updated to latest commit 8af66f7

[github-actions]

Persistent review updated to latest commit 70787c6

[github-actions]

Persistent review updated to latest commit 83b65c4

[github-actions]

Persistent review updated to latest commit a435c53