karuppayya commented on code in PR #12450: URL: https://github.com/apache/iceberg/pull/12450#discussion_r2093404118
########## api/src/main/java/org/apache/iceberg/actions/ComputePartitionStats.java: ########## @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.actions; + +import org.apache.iceberg.PartitionStatisticsFile; + +/** + * An action that computes and writes the partition statistics of an Iceberg table. Current snapshot + * is used by default. + */ +public interface ComputePartitionStats + extends Action<ComputePartitionStats, ComputePartitionStats.Result> { + /** + * Choose the table snapshot to compute partition stats. + * + * @param snapshotId long ID of the snapshot for which stats need to be computed + * @return this for method chaining + */ + ComputePartitionStats snapshot(long snapshotId); Review Comment: I think the action is the delegate for the Procedure. Procedure does input validation and result(from action's execution) conversion to internal row. The business logic should remain in action. In procedure, we would set the boolean to force refresh on the action, and action decides how it will use it. ########## api/src/main/java/org/apache/iceberg/actions/ComputePartitionStats.java: ########## @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.iceberg.actions; + +import org.apache.iceberg.PartitionStatisticsFile; + +/** + * An action that computes and writes the partition statistics of an Iceberg table. Current snapshot + * is used by default. + */ +public interface ComputePartitionStats + extends Action<ComputePartitionStats, ComputePartitionStats.Result> { Review Comment: Thanks @amogh-jahagirdar for bringing this up. Though we don't need to follow other engines, i can see examples where `ANALYZE TABLE` is used for both table level and partition level stats. 1. [Hive](https://cwiki.apache.org/confluence/display/hive/statsdev#StatsDev-Examples) 2. [Spark](https://spark.apache.org/docs/latest/sql-ref-syntax-aux-analyze-table.html) 3. [Trino](https://trino.io/docs/current/sql/analyze.html#examples) I think it would be good to follow a similar pattern for Iceberg actions and procedure, so that users don't have to think about gathering iceberg stats differently as against other engines. But like highlighted, this comes with a complexity of adding conditional code. @ajantha-bhat , i think the force refresh is something that would be useful for current table stats action as well. Also, eventually we would need to support incremental stats with the current table stats as well. -- 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]
