Copilot commented on code in PR #12011:
URL: https://github.com/apache/gluten/pull/12011#discussion_r3160243375


##########
cpp-ch/local-engine/Storages/SubstraitSource/ParquetFormatFile.cpp:
##########
@@ -107,8 +110,7 @@ class ParquetInputFormat : public FormatFile::InputFormat
         {
             assert(outputHeader.columns());
             assert(row_index_reader);
-            // TODO: rebase-25.12, format_settings_.parquet.max_block_size
-            Columns cols{row_index_reader->readBatch(8192)};
+            Columns cols{row_index_reader->readBatch(max_block_size)};
             size_t rows = cols[0]->size();
             return Chunk(std::move(cols), rows);

Review Comment:
   This change alters the batch size used when only meta columns are requested 
(readHeader.columns()==0) to depend on 
`format_settings.parquet.max_block_size`. There doesn’t appear to be a 
unit/integration test covering that row-index-only path with a non-default 
`max_block_size`; adding one would help prevent regressions (e.g., ensuring 
chunk sizes follow the setting rather than a hardcoded constant).



##########
backends-clickhouse/src/main/scala/org/apache/gluten/backendsapi/clickhouse/CHListenerApi.scala:
##########
@@ -90,8 +90,7 @@ class CHListenerApi extends ListenerApi with Logging {
     // Add configs
     import org.apache.gluten.backendsapi.clickhouse.CHConfig._
     conf.setCHConfig(
-      "timezone" -> conf.get("spark.sql.session.timeZone", 
TimeZone.getDefault.getID),
-      "local_engine.settings.log_processors_profiles" -> "true")
+      "timezone" -> conf.get("spark.sql.session.timeZone", 
TimeZone.getDefault.getID))
     conf.setCHSettings("spark_version", SPARK_VERSION)

Review Comment:
   The PR description/title is about passing correct values to 
ParquetInputFormat, but this hunk also removes the default CH setting 
`local_engine.settings.log_processors_profiles = true`. If this behavior change 
is intended, it should be called out in the PR description (or moved to a 
separate PR) since it changes runtime logging/profiling defaults for the 
ClickHouse backend.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to