You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "reswqa (via GitHub)" <gi...@apache.org> on 2024/08/27 09:03:56 UTC
[PR] [flink] Implements FlinkCatalog#getPartition [paimon]
reswqa opened a new pull request, #4073:
URL: https://github.com/apache/paimon/pull/4073
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
Implements `FlinkCatalog#getPartition`.
### Tests
IT
### API and Format
No.
### Documentation
No.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #4073:
URL: https://github.com/apache/paimon/pull/4073#issuecomment-2316569724
> @reswqa Is there any SQL will invoke this method?
Currently, no..
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4073:
URL: https://github.com/apache/paimon/pull/4073#discussion_r1732529735
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -126,12 +128,16 @@
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.nonPhysicalColumnsCount;
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.serializeNewWatermarkSpec;
import static org.apache.paimon.utils.Preconditions.checkArgument;
+import static org.apache.paimon.utils.Preconditions.checkNotNull;
/** Catalog for paimon. */
public class FlinkCatalog extends AbstractCatalog {
private static final Logger LOG = LoggerFactory.getLogger(FlinkCatalog.class);
-
+ public static final String NUM_ROWS_KEY = "partition.numRows";
Review Comment:
I suggest establishing a standard under the definition of the Flink community.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #4073:
URL: https://github.com/apache/paimon/pull/4073#issuecomment-2316555629
@reswqa Is there any SQL will invoke this method?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4073:
URL: https://github.com/apache/paimon/pull/4073#discussion_r1732916035
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -922,7 +928,34 @@ public final List<CatalogPartitionSpec> listPartitionsByFilter(
@Override
public CatalogPartition getPartition(ObjectPath tablePath, CatalogPartitionSpec partitionSpec)
throws PartitionNotExistException, CatalogException {
- throw new PartitionNotExistException(getName(), tablePath, partitionSpec);
+ checkNotNull(partitionSpec, "partition spec shouldn't be null");
+ Identifier identifier = toIdentifier(tablePath);
+ try {
+ Table table = catalog.getTable(identifier);
Review Comment:
Can you just extract a method for reusing codes from `getPartitionSpecs`?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #4073:
URL: https://github.com/apache/paimon/pull/4073
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #4073:
URL: https://github.com/apache/paimon/pull/4073#discussion_r1732506099
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -922,7 +928,36 @@ public final List<CatalogPartitionSpec> listPartitionsByFilter(
@Override
public CatalogPartition getPartition(ObjectPath tablePath, CatalogPartitionSpec partitionSpec)
throws PartitionNotExistException, CatalogException {
- throw new PartitionNotExistException(getName(), tablePath, partitionSpec);
+ checkNotNull(partitionSpec, "partition spec shouldn't be null");
+ Identifier identifier = toIdentifier(tablePath);
+ try {
+ Table table = catalog().getTable(identifier);
+ FileStoreTable fileStoreTable = (FileStoreTable) table;
Review Comment:
Why cast to FileStoreTable? Just use Table
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -922,7 +928,36 @@ public final List<CatalogPartitionSpec> listPartitionsByFilter(
@Override
public CatalogPartition getPartition(ObjectPath tablePath, CatalogPartitionSpec partitionSpec)
throws PartitionNotExistException, CatalogException {
- throw new PartitionNotExistException(getName(), tablePath, partitionSpec);
+ checkNotNull(partitionSpec, "partition spec shouldn't be null");
+ Identifier identifier = toIdentifier(tablePath);
+ try {
+ Table table = catalog().getTable(identifier);
Review Comment:
catalog.get...
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -922,7 +928,36 @@ public final List<CatalogPartitionSpec> listPartitionsByFilter(
@Override
public CatalogPartition getPartition(ObjectPath tablePath, CatalogPartitionSpec partitionSpec)
throws PartitionNotExistException, CatalogException {
- throw new PartitionNotExistException(getName(), tablePath, partitionSpec);
+ checkNotNull(partitionSpec, "partition spec shouldn't be null");
+ Identifier identifier = toIdentifier(tablePath);
+ try {
+ Table table = catalog().getTable(identifier);
+ FileStoreTable fileStoreTable = (FileStoreTable) table;
+
+ if (fileStoreTable.partitionKeys() == null
+ || fileStoreTable.partitionKeys().size() == 0) {
+ throw new PartitionNotExistException(getName(), tablePath, partitionSpec);
+ }
+
+ ReadBuilder readBuilder = table.newReadBuilder();
Review Comment:
One line is ok:
ReadBuilder readBuilder = table.newReadBuilder().withPartitionFilter(checkNotNull(partitionSpec.getPartitionSpec()));
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "lsyldliu (via GitHub)" <gi...@apache.org>.
lsyldliu commented on code in PR #4073:
URL: https://github.com/apache/paimon/pull/4073#discussion_r1734378378
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -126,12 +127,16 @@
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.nonPhysicalColumnsCount;
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.serializeNewWatermarkSpec;
import static org.apache.paimon.utils.Preconditions.checkArgument;
+import static org.apache.paimon.utils.Preconditions.checkNotNull;
/** Catalog for paimon. */
public class FlinkCatalog extends AbstractCatalog {
private static final Logger LOG = LoggerFactory.getLogger(FlinkCatalog.class);
-
+ public static final String NUM_ROWS_KEY = "numRows";
+ public static final String LAST_UPDATE_TIME_KEY = "lastUpdateTime";
+ public static final String FILE_SIZE_IN_BYTES_KEY = "fileSizeInBytes";
+ public static final String FILE_COUNT_KEY = "fileCount";
Review Comment:
numFiles? Hive name it.
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -126,12 +127,16 @@
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.nonPhysicalColumnsCount;
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.serializeNewWatermarkSpec;
import static org.apache.paimon.utils.Preconditions.checkArgument;
+import static org.apache.paimon.utils.Preconditions.checkNotNull;
/** Catalog for paimon. */
public class FlinkCatalog extends AbstractCatalog {
private static final Logger LOG = LoggerFactory.getLogger(FlinkCatalog.class);
-
+ public static final String NUM_ROWS_KEY = "numRows";
+ public static final String LAST_UPDATE_TIME_KEY = "lastUpdateTime";
+ public static final String FILE_SIZE_IN_BYTES_KEY = "fileSizeInBytes";
Review Comment:
Hive is `totalSize`, does this options is represent the same means of hive?
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -126,12 +127,16 @@
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.nonPhysicalColumnsCount;
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.serializeNewWatermarkSpec;
import static org.apache.paimon.utils.Preconditions.checkArgument;
+import static org.apache.paimon.utils.Preconditions.checkNotNull;
/** Catalog for paimon. */
public class FlinkCatalog extends AbstractCatalog {
private static final Logger LOG = LoggerFactory.getLogger(FlinkCatalog.class);
-
+ public static final String NUM_ROWS_KEY = "partition.numRows";
Review Comment:
+1, As discussed offline, we need to standardize these parameters in the Flink community. And we don't need the prefix of partition, these parameters may represent partition level or table level, as Hive does.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [flink] Implements FlinkCatalog#getPartition [paimon]
Posted by "reswqa (via GitHub)" <gi...@apache.org>.
reswqa commented on code in PR #4073:
URL: https://github.com/apache/paimon/pull/4073#discussion_r1734404681
##########
paimon-flink/paimon-flink-common/src/main/java/org/apache/paimon/flink/FlinkCatalog.java:
##########
@@ -126,12 +127,16 @@
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.nonPhysicalColumnsCount;
import static org.apache.paimon.flink.utils.FlinkCatalogPropertiesUtil.serializeNewWatermarkSpec;
import static org.apache.paimon.utils.Preconditions.checkArgument;
+import static org.apache.paimon.utils.Preconditions.checkNotNull;
/** Catalog for paimon. */
public class FlinkCatalog extends AbstractCatalog {
private static final Logger LOG = LoggerFactory.getLogger(FlinkCatalog.class);
-
+ public static final String NUM_ROWS_KEY = "partition.numRows";
Review Comment:
Removed.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org