core/src/main/java/io/questdb/griffin/engine/groupby/SampleByInterpolateRecordCursorFactory.java (1)

374-377: Consider moving sampleFromFunc.init() and sampleToFunc.init() into parseParams() for consistency.

parseParams() already initializes timezoneNameFunc and offsetFunc. Initializing the new functions outside of it breaks this pattern and could be missed if parseParams() is overridden in a subclass.

♻️ Proposed refactor — consolidate into parseParams()

         parseParams(this, executionContext);
-        sampleFromFunc.init(managedCursor, executionContext);
-        sampleToFunc.init(managedCursor, executionContext);
         areTimestampsInitialized = false;

In parseParams():

     protected void parseParams(RecordCursor base, SqlExecutionContext executionContext) throws SqlException {
         timezoneNameFunc.init(base, executionContext);
         offsetFunc.init(base, executionContext);
+        sampleFromFunc.init(base, executionContext);
+        sampleToFunc.init(base, executionContext);
         rules = null;

🤖 Prompt for AI Agents

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

In
`@core/src/main/java/io/questdb/griffin/engine/groupby/SampleByInterpolateRecordCursorFactory.java`
around lines 374 - 377, parseParams currently initializes timezoneNameFunc and
offsetFunc but sampleFromFunc.init(...) and sampleToFunc.init(...) are called
externally; move those two init calls into parseParams so all function
initializations are consistent and won't be missed by subclasses overriding
parseParams. Update parseParams to accept (or access) managedCursor and
executionContext as needed, call sampleFromFunc.init(managedCursor,
executionContext) and sampleToFunc.init(managedCursor, executionContext) there,
remove the duplicate init calls from the caller (the block that sets
areTimestampsInitialized = false), and ensure any subclasses of parseParams are
updated if they override it.

core/src/test/java/io/questdb/test/griffin/engine/groupby/SampleByTest.java (1)

6044-6044: Use underscore thousands separator in 60000T.

60000T appears on lines 6044, 6057, 6072, and 6352. Per coding guidelines, numbers with 5 or more digits must use underscores as thousands separators.

Proposed fix (apply at all four locations)

-            SAMPLE BY 60000T
+            SAMPLE BY 60_000T

As per coding guidelines: "Always use underscores as thousands separators in numbers of 5 digits or more in QuestDB SQL and implementation code."

Also applies to: 6057-6057, 6072-6072, 6352-6352

🤖 Prompt for AI Agents

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

In `@core/src/test/java/io/questdb/test/griffin/engine/groupby/SampleByTest.java`
at line 6044, Replace the numeric literal using five digits without separators
in the SAMPLE BY clauses: change every "SAMPLE BY 60000T" occurrence in
SampleByTest (the SQL/text literal "SAMPLE BY 60000T") to use an underscore
thousands separator ("SAMPLE BY 60_000T"); apply the same replacement for all
other instances of 60000T in this file so numbers with 5+ digits follow the
underscore separator rule.