You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "codope (via GitHub)" <gi...@apache.org> on 2024/09/17 16:03:55 UTC
[PR] [HUDI-7563] Add support to drop index using sql [hudi]
codope opened a new pull request, #11951:
URL: https://github.com/apache/hudi/pull/11951
### Change Logs
- Support dropping index through SQL
- Add a test to drop sec index
- Some renames and refactoring
### Impact
Users can now drop index using SQL
### Risk level (write none, low medium or high below)
low
### Documentation Update
_Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none"._
- _The config description must be updated if new configs are added or the default value of the configs are changed_
- _Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the [instruction](https://hudi.apache.org/contribute/developer-setup#website) to make
changes to the website._
### Contributor's checklist
- [ ] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [ ] Change Logs and Impact were stated clearly
- [ ] Adequate tests were added if applicable
- [ ] CI passed
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2356525509
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 851ed023ddb4064077e7216e2b926d7fe0bf39e0 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517)
* 5fc84344609288c5717f7b530856ec2fff910cac UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2356351261
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 851ed023ddb4064077e7216e2b926d7fe0bf39e0 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2356348709
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 851ed023ddb4064077e7216e2b926d7fe0bf39e0 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2365007857
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
}, {
"hash" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652",
"triggerID" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 235756dc9217240bedf2317ba134b4b10e96e72d Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2365023625
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
}, {
"hash" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652",
"triggerID" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"triggerType" : "PUSH"
}, {
"hash" : "e8a0631fdadef4f8f4b054924f6784f566427c05",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=660",
"triggerID" : "e8a0631fdadef4f8f4b054924f6784f566427c05",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 235756dc9217240bedf2317ba134b4b10e96e72d Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652)
* e8a0631fdadef4f8f4b054924f6784f566427c05 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=660)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769485575
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/IndexCommands.scala:
##########
@@ -76,7 +76,14 @@ case class DropIndexCommand(table: CatalogTable,
override def run(sparkSession: SparkSession): Seq[Row] = {
val tableId = table.identifier
val metaClient = createHoodieTableMetaClient(tableId, sparkSession)
- SecondaryIndexManager.getInstance().drop(metaClient, indexName, ignoreIfNotExists)
+ try {
+ // need to ensure that the index name is for a valid partition type
+ MetadataPartitionType.fromPartitionPath(indexName)
+ HoodieSparkIndexClient.getInstance(sparkSession).drop(metaClient, indexName, ignoreIfNotExists)
+ } catch {
+ case _: IllegalArgumentException =>
+ SecondaryIndexManager.getInstance().drop(metaClient, indexName, ignoreIfNotExists)
Review Comment:
This is legacy code due to incomplete RFC-52. `SecondaryIndexManager` was introduced in [RFC-52](https://github.com/apache/hudi/pull/5370) but it's just a wrapper code and does not really manage any index underneath. From RFC, it's supposed to support index built on third party libraries such as Lucene. But, we have not yet added any support so far. In my opinion, we should remove all that code. Just keeping it here to make some tests pass. If you agree, I can take the cleanup as a followup later.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2359224600
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0ea02b94211fcd71a660fd46670d1acb1ce591a0 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2364988405
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
}, {
"hash" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652",
"triggerID" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0ea02b94211fcd71a660fd46670d1acb1ce591a0 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545)
* 235756dc9217240bedf2317ba134b4b10e96e72d Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua merged PR #11951:
URL: https://github.com/apache/hudi/pull/11951
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2359072740
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5fc84344609288c5717f7b530856ec2fff910cac Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520)
* 0ea02b94211fcd71a660fd46670d1acb1ce591a0 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2356507854
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 851ed023ddb4064077e7216e2b926d7fe0bf39e0 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769466010
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/HoodieSparkIndexClient.java:
##########
@@ -56,24 +56,24 @@
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX;
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX_PREFIX;
-public class HoodieSparkFunctionalIndexClient extends BaseHoodieFunctionalIndexClient {
+public class HoodieSparkIndexClient extends BaseHoodieIndexClient {
- private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkFunctionalIndexClient.class);
+ private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkIndexClient.class);
- private static volatile HoodieSparkFunctionalIndexClient _instance;
+ private static volatile HoodieSparkIndexClient _instance;
private final SparkSession sparkSession;
- private HoodieSparkFunctionalIndexClient(SparkSession sparkSession) {
+ private HoodieSparkIndexClient(SparkSession sparkSession) {
super();
this.sparkSession = sparkSession;
}
- public static HoodieSparkFunctionalIndexClient getInstance(SparkSession sparkSession) {
+ public static HoodieSparkIndexClient getInstance(SparkSession sparkSession) {
if (_instance == null) {
- synchronized (HoodieSparkFunctionalIndexClient.class) {
+ synchronized (HoodieSparkIndexClient.class) {
if (_instance == null) {
- _instance = new HoodieSparkFunctionalIndexClient(sparkSession);
+ _instance = new HoodieSparkIndexClient(sparkSession);
Review Comment:
This logic is a bit weird for concurrency control. We should revisit it later.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java:
##########
@@ -61,4 +62,9 @@ public void register(HoodieTableMetaClient metaClient, String indexName, String
* Create a functional index.
*/
public abstract void create(HoodieTableMetaClient metaClient, String indexName, String indexType, Map<String, Map<String, String>> columns, Map<String, String> options);
+
+ /**
+ * Drop an index. By default, ignore drop if index does not exist.
+ */
+ public abstract void drop(HoodieTableMetaClient metaClient, String indexName, MetadataPartitionType metadataPartitionType, boolean ignoreIfNotExists);
Review Comment:
+1, could you consolidate `indexName` and `metadataPartitionType` into one argument?
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java:
##########
@@ -61,4 +62,9 @@ public void register(HoodieTableMetaClient metaClient, String indexName, String
* Create a functional index.
*/
public abstract void create(HoodieTableMetaClient metaClient, String indexName, String indexType, Map<String, Map<String, String>> columns, Map<String, String> options);
+
+ /**
+ * Drop an index. By default, ignore drop if index does not exist.
+ */
+ public abstract void drop(HoodieTableMetaClient metaClient, String indexName, MetadataPartitionType metadataPartitionType, boolean ignoreIfNotExists);
Review Comment:
Is there a case we need to set different `ignoreIfNotExists`? If not, can we remove the argument too?
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769484635
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/HoodieSparkIndexClient.java:
##########
@@ -56,24 +56,24 @@
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX;
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX_PREFIX;
-public class HoodieSparkFunctionalIndexClient extends BaseHoodieFunctionalIndexClient {
+public class HoodieSparkIndexClient extends BaseHoodieIndexClient {
- private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkFunctionalIndexClient.class);
+ private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkIndexClient.class);
- private static volatile HoodieSparkFunctionalIndexClient _instance;
+ private static volatile HoodieSparkIndexClient _instance;
private final SparkSession sparkSession;
- private HoodieSparkFunctionalIndexClient(SparkSession sparkSession) {
+ private HoodieSparkIndexClient(SparkSession sparkSession) {
super();
this.sparkSession = sparkSession;
}
- public static HoodieSparkFunctionalIndexClient getInstance(SparkSession sparkSession) {
+ public static HoodieSparkIndexClient getInstance(SparkSession sparkSession) {
if (_instance == null) {
- synchronized (HoodieSparkFunctionalIndexClient.class) {
+ synchronized (HoodieSparkIndexClient.class) {
if (_instance == null) {
- _instance = new HoodieSparkFunctionalIndexClient(sparkSession);
+ _instance = new HoodieSparkIndexClient(sparkSession);
Review Comment:
This is the usual double-checked locking pattern. Probably you are concerned about memory visibility issues. The `_instance` could be non-null and yet not fully constructed when the first `if (_instance == null)` check passes, but before the object is fully initialized. However, note that `_instance` is declared as `volatile` which ensures that any writes to `_instance` are visible to all threads immediately and prevents the reordering of instructions during construction.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1772377686
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/HoodieSparkIndexClient.java:
##########
@@ -112,6 +112,24 @@ public void create(HoodieTableMetaClient metaClient, String indexName, String in
}
}
+ @Override
+ public void drop(HoodieTableMetaClient metaClient, String indexName, boolean ignoreIfNotExists) {
+ if (!indexExists(metaClient, indexName)) {
+ if (ignoreIfNotExists) {
+ return;
+ } else {
+ throw new HoodieFunctionalIndexException("Index does not exist: " + indexName);
+ }
+ }
+
+ LOG.info("Dropping index {}", indexName);
+ HoodieIndexDefinition indexDefinition = metaClient.getIndexMetadata().get().getIndexDefinitions().get(indexName);
+ try (SparkRDDWriteClient writeClient = HoodieCLIUtils.createHoodieWriteClient(
+ sparkSession, metaClient.getBasePath().toString(), mapAsScalaImmutableMap(buildWriteConfig(metaClient, indexDefinition)), toScalaOption(Option.empty()))) {
+ writeClient.dropIndex(Collections.singletonList(indexName));
Review Comment:
There is `BaseHoodieWriteClient` to abstract the write logic. Engine-specific implementation only needs to instantiate the engine-specific write client. The index client should only care about calling APIs in `BaseHoodieWriteClient`, which is the case here (need to generalize `HoodieCLIUtils.createHoodieWriteClient` too).
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1772373645
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/HoodieSparkIndexClient.java:
##########
@@ -56,24 +56,24 @@
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX;
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX_PREFIX;
-public class HoodieSparkFunctionalIndexClient extends BaseHoodieFunctionalIndexClient {
+public class HoodieSparkIndexClient extends BaseHoodieIndexClient {
- private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkFunctionalIndexClient.class);
+ private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkIndexClient.class);
- private static volatile HoodieSparkFunctionalIndexClient _instance;
+ private static volatile HoodieSparkIndexClient _instance;
private final SparkSession sparkSession;
- private HoodieSparkFunctionalIndexClient(SparkSession sparkSession) {
+ private HoodieSparkIndexClient(SparkSession sparkSession) {
super();
this.sparkSession = sparkSession;
}
- public static HoodieSparkFunctionalIndexClient getInstance(SparkSession sparkSession) {
+ public static HoodieSparkIndexClient getInstance(SparkSession sparkSession) {
if (_instance == null) {
- synchronized (HoodieSparkFunctionalIndexClient.class) {
+ synchronized (HoodieSparkIndexClient.class) {
if (_instance == null) {
- _instance = new HoodieSparkFunctionalIndexClient(sparkSession);
+ _instance = new HoodieSparkIndexClient(sparkSession);
Review Comment:
OK sg. For simplicity I'm thinking if this method can be made synchronized to avoid the logic. It's minor.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2359070501
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5fc84344609288c5717f7b530856ec2fff910cac Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520)
* 0ea02b94211fcd71a660fd46670d1acb1ce591a0 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2365047966
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
}, {
"hash" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652",
"triggerID" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"triggerType" : "PUSH"
}, {
"hash" : "e8a0631fdadef4f8f4b054924f6784f566427c05",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=660",
"triggerID" : "e8a0631fdadef4f8f4b054924f6784f566427c05",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e8a0631fdadef4f8f4b054924f6784f566427c05 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=660)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769468772
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/hudi/functional/TestSecondaryIndexPruning.scala:
##########
@@ -158,6 +158,70 @@ class TestSecondaryIndexPruning extends SparkClientFunctionalTestHarness {
}
}
+ @ParameterizedTest
+ @MethodSource(Array("testSecondaryIndexPruningParameters"))
Review Comment:
Since dropping index isn't coupled with table type and partitioning, testing one combination is good enough.
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/command/index/TestFunctionalIndex.scala:
##########
@@ -248,6 +249,86 @@ class TestFunctionalIndex extends HoodieSparkSqlTestBase {
}
}
+ test("Test Drop Functional Index") {
+ if (HoodieSparkUtils.gteqSpark3_3) {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
Review Comment:
Similar here on test combinations
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/command/index/TestFunctionalIndex.scala:
##########
@@ -248,6 +249,86 @@ class TestFunctionalIndex extends HoodieSparkSqlTestBase {
}
}
+ test("Test Drop Functional Index") {
+ if (HoodieSparkUtils.gteqSpark3_3) {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val databaseName = "default"
+ val tableName = generateTableName
+ val basePath = s"${tmp.getCanonicalPath}/$tableName"
+ spark.sql(
+ s"""
+ |create table $tableName (
+ | id int,
+ | name string,
+ | price double,
+ | ts long
+ |) using hudi
+ | options (
+ | primaryKey ='id',
+ | type = '$tableType',
+ | preCombineField = 'ts',
+ | hoodie.metadata.record.index.enable = 'true',
+ | hoodie.datasource.write.recordkey.field = 'id'
+ | )
+ | partitioned by(ts)
+ | location '$basePath'
+ """.stripMargin)
+ spark.sql(s"insert into $tableName values(1, 'a1', 10, 1000)")
+ spark.sql(s"insert into $tableName values(2, 'a2', 10, 1001)")
+ spark.sql(s"insert into $tableName values(3, 'a3', 10, 1002)")
+
+ var metaClient = createMetaClient(spark, basePath)
+
+ assert(metaClient.getTableConfig.isMetadataPartitionAvailable(MetadataPartitionType.RECORD_INDEX))
+
+ val sqlParser: ParserInterface = spark.sessionState.sqlParser
+ val analyzer: Analyzer = spark.sessionState.analyzer
+
+ var logicalPlan = sqlParser.parsePlan(s"show indexes from default.$tableName")
+ var resolvedLogicalPlan = analyzer.execute(logicalPlan)
+ assertTableIdentifier(resolvedLogicalPlan.asInstanceOf[ShowIndexesCommand].table, databaseName, tableName)
+
+ var createIndexSql = s"create index idx_datestr on $tableName using column_stats(ts) options(func='from_unixtime', format='yyyy-MM-dd')"
+ logicalPlan = sqlParser.parsePlan(createIndexSql)
+
+ resolvedLogicalPlan = analyzer.execute(logicalPlan)
+ assertTableIdentifier(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].table, databaseName, tableName)
+ assertResult("idx_datestr")(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].indexName)
+ assertResult("column_stats")(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].indexType)
+ assertResult(false)(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].ignoreIfExists)
Review Comment:
Can some of the validation logic be simplified and removed since this test only targets "DROP INDEX"?
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/IndexCommands.scala:
##########
@@ -76,7 +76,14 @@ case class DropIndexCommand(table: CatalogTable,
override def run(sparkSession: SparkSession): Seq[Row] = {
val tableId = table.identifier
val metaClient = createHoodieTableMetaClient(tableId, sparkSession)
- SecondaryIndexManager.getInstance().drop(metaClient, indexName, ignoreIfNotExists)
+ try {
+ // need to ensure that the index name is for a valid partition type
+ MetadataPartitionType.fromPartitionPath(indexName)
+ HoodieSparkIndexClient.getInstance(sparkSession).drop(metaClient, indexName, ignoreIfNotExists)
+ } catch {
+ case _: IllegalArgumentException =>
+ SecondaryIndexManager.getInstance().drop(metaClient, indexName, ignoreIfNotExists)
Review Comment:
Why drop here again?
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/HoodieSparkIndexClient.java:
##########
@@ -112,6 +112,24 @@ public void create(HoodieTableMetaClient metaClient, String indexName, String in
}
}
+ @Override
+ public void drop(HoodieTableMetaClient metaClient, String indexName, boolean ignoreIfNotExists) {
+ if (!indexExists(metaClient, indexName)) {
+ if (ignoreIfNotExists) {
+ return;
+ } else {
+ throw new HoodieFunctionalIndexException("Index does not exist: " + indexName);
+ }
+ }
+
+ LOG.info("Dropping index {}", indexName);
+ HoodieIndexDefinition indexDefinition = metaClient.getIndexMetadata().get().getIndexDefinitions().get(indexName);
+ try (SparkRDDWriteClient writeClient = HoodieCLIUtils.createHoodieWriteClient(
+ sparkSession, metaClient.getBasePath().toString(), mapAsScalaImmutableMap(buildWriteConfig(metaClient, indexDefinition)), toScalaOption(Option.empty()))) {
+ writeClient.dropIndex(Collections.singletonList(indexName));
Review Comment:
For future reference, this logic does not have much specifics to engine itself, so it can be abstracted to the index client by plugging in the engine-specific write client.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2365023252
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
}, {
"hash" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652",
"triggerID" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"triggerType" : "PUSH"
}, {
"hash" : "e8a0631fdadef4f8f4b054924f6784f566427c05",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "e8a0631fdadef4f8f4b054924f6784f566427c05",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 235756dc9217240bedf2317ba134b4b10e96e72d Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=652)
* e8a0631fdadef4f8f4b054924f6784f566427c05 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2364987812
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
}, {
"hash" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545",
"triggerID" : "0ea02b94211fcd71a660fd46670d1acb1ce591a0",
"triggerType" : "PUSH"
}, {
"hash" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "235756dc9217240bedf2317ba134b4b10e96e72d",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 0ea02b94211fcd71a660fd46670d1acb1ce591a0 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=545)
* 235756dc9217240bedf2317ba134b4b10e96e72d UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769484179
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java:
##########
@@ -61,4 +62,9 @@ public void register(HoodieTableMetaClient metaClient, String indexName, String
* Create a functional index.
*/
public abstract void create(HoodieTableMetaClient metaClient, String indexName, String indexType, Map<String, Map<String, String>> columns, Map<String, String> options);
+
+ /**
+ * Drop an index. By default, ignore drop if index does not exist.
+ */
+ public abstract void drop(HoodieTableMetaClient metaClient, String indexName, MetadataPartitionType metadataPartitionType, boolean ignoreIfNotExists);
Review Comment:
I have removed `metadataPartitionType` argument. On `ignoreIfNotExists`, we support two types of DROP INDEX statements:
```
DROP INDEX <index_name> ON <table_name>;
DROP INDEX IF EXISTS <index_name> ON <table_name>;
```
In the first case `ignoreIfNotExists` is false while true in the latter case. So, in the sql prompt, the error will be thrown if ignore is set to false so that user knows that the index already doesn't exist. Just need this to support different sql flavors.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2356646033
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "DELETED",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5fc84344609288c5717f7b530856ec2fff910cac Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #11951:
URL: https://github.com/apache/hudi/pull/11951#issuecomment-2356529094
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"status" : "FAILURE",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517",
"triggerID" : "851ed023ddb4064077e7216e2b926d7fe0bf39e0",
"triggerType" : "PUSH"
}, {
"hash" : "5fc84344609288c5717f7b530856ec2fff910cac",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520",
"triggerID" : "5fc84344609288c5717f7b530856ec2fff910cac",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 851ed023ddb4064077e7216e2b926d7fe0bf39e0 Azure: [FAILURE](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=517)
* 5fc84344609288c5717f7b530856ec2fff910cac Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=520)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769485614
##########
hudi-spark-datasource/hudi-spark/src/test/scala/org/apache/spark/sql/hudi/command/index/TestFunctionalIndex.scala:
##########
@@ -248,6 +249,86 @@ class TestFunctionalIndex extends HoodieSparkSqlTestBase {
}
}
+ test("Test Drop Functional Index") {
+ if (HoodieSparkUtils.gteqSpark3_3) {
+ withTempDir { tmp =>
+ Seq("cow", "mor").foreach { tableType =>
+ val databaseName = "default"
+ val tableName = generateTableName
+ val basePath = s"${tmp.getCanonicalPath}/$tableName"
+ spark.sql(
+ s"""
+ |create table $tableName (
+ | id int,
+ | name string,
+ | price double,
+ | ts long
+ |) using hudi
+ | options (
+ | primaryKey ='id',
+ | type = '$tableType',
+ | preCombineField = 'ts',
+ | hoodie.metadata.record.index.enable = 'true',
+ | hoodie.datasource.write.recordkey.field = 'id'
+ | )
+ | partitioned by(ts)
+ | location '$basePath'
+ """.stripMargin)
+ spark.sql(s"insert into $tableName values(1, 'a1', 10, 1000)")
+ spark.sql(s"insert into $tableName values(2, 'a2', 10, 1001)")
+ spark.sql(s"insert into $tableName values(3, 'a3', 10, 1002)")
+
+ var metaClient = createMetaClient(spark, basePath)
+
+ assert(metaClient.getTableConfig.isMetadataPartitionAvailable(MetadataPartitionType.RECORD_INDEX))
+
+ val sqlParser: ParserInterface = spark.sessionState.sqlParser
+ val analyzer: Analyzer = spark.sessionState.analyzer
+
+ var logicalPlan = sqlParser.parsePlan(s"show indexes from default.$tableName")
+ var resolvedLogicalPlan = analyzer.execute(logicalPlan)
+ assertTableIdentifier(resolvedLogicalPlan.asInstanceOf[ShowIndexesCommand].table, databaseName, tableName)
+
+ var createIndexSql = s"create index idx_datestr on $tableName using column_stats(ts) options(func='from_unixtime', format='yyyy-MM-dd')"
+ logicalPlan = sqlParser.parsePlan(createIndexSql)
+
+ resolvedLogicalPlan = analyzer.execute(logicalPlan)
+ assertTableIdentifier(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].table, databaseName, tableName)
+ assertResult("idx_datestr")(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].indexName)
+ assertResult("column_stats")(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].indexType)
+ assertResult(false)(resolvedLogicalPlan.asInstanceOf[CreateIndexCommand].ignoreIfExists)
Review Comment:
makes sense as we're validating the plan in another test.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769484635
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/HoodieSparkIndexClient.java:
##########
@@ -56,24 +56,24 @@
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX;
import static org.apache.hudi.metadata.HoodieTableMetadataUtil.PARTITION_NAME_SECONDARY_INDEX_PREFIX;
-public class HoodieSparkFunctionalIndexClient extends BaseHoodieFunctionalIndexClient {
+public class HoodieSparkIndexClient extends BaseHoodieIndexClient {
- private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkFunctionalIndexClient.class);
+ private static final Logger LOG = LoggerFactory.getLogger(HoodieSparkIndexClient.class);
- private static volatile HoodieSparkFunctionalIndexClient _instance;
+ private static volatile HoodieSparkIndexClient _instance;
private final SparkSession sparkSession;
- private HoodieSparkFunctionalIndexClient(SparkSession sparkSession) {
+ private HoodieSparkIndexClient(SparkSession sparkSession) {
super();
this.sparkSession = sparkSession;
}
- public static HoodieSparkFunctionalIndexClient getInstance(SparkSession sparkSession) {
+ public static HoodieSparkIndexClient getInstance(SparkSession sparkSession) {
if (_instance == null) {
- synchronized (HoodieSparkFunctionalIndexClient.class) {
+ synchronized (HoodieSparkIndexClient.class) {
if (_instance == null) {
- _instance = new HoodieSparkFunctionalIndexClient(sparkSession);
+ _instance = new HoodieSparkIndexClient(sparkSession);
Review Comment:
This is the usual double-checked locking pattern. Probably you are concerned about memory visibility issues. The `_instance` could be non-null and yet not fully constructed when the first `if (_instance == null)` check passes, but before the object is fully initialized. However, note that `instance` is declared as `volatile` which ensures that any writes to `_instance` are visible to all threads immediately and prevents the reordering of instructions during construction.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769485704
##########
hudi-spark-datasource/hudi-spark-common/src/main/java/org/apache/hudi/HoodieSparkIndexClient.java:
##########
@@ -112,6 +112,24 @@ public void create(HoodieTableMetaClient metaClient, String indexName, String in
}
}
+ @Override
+ public void drop(HoodieTableMetaClient metaClient, String indexName, boolean ignoreIfNotExists) {
+ if (!indexExists(metaClient, indexName)) {
+ if (ignoreIfNotExists) {
+ return;
+ } else {
+ throw new HoodieFunctionalIndexException("Index does not exist: " + indexName);
+ }
+ }
+
+ LOG.info("Dropping index {}", indexName);
+ HoodieIndexDefinition indexDefinition = metaClient.getIndexMetadata().get().getIndexDefinitions().get(indexName);
+ try (SparkRDDWriteClient writeClient = HoodieCLIUtils.createHoodieWriteClient(
+ sparkSession, metaClient.getBasePath().toString(), mapAsScalaImmutableMap(buildWriteConfig(metaClient, indexDefinition)), toScalaOption(Option.empty()))) {
+ writeClient.dropIndex(Collections.singletonList(indexName));
Review Comment:
Don't we need the engine-specific write client to call the base API `BaseHoodieWriteClient.dropIndex`?
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1769274191
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java:
##########
@@ -61,4 +62,9 @@ public void register(HoodieTableMetaClient metaClient, String indexName, String
* Create a functional index.
*/
public abstract void create(HoodieTableMetaClient metaClient, String indexName, String indexType, Map<String, Map<String, String>> columns, Map<String, String> options);
+
+ /**
+ * Drop an index. By default, ignore drop if index does not exist.
+ */
+ public abstract void drop(HoodieTableMetaClient metaClient, String indexName, MetadataPartitionType metadataPartitionType, boolean ignoreIfNotExists);
Review Comment:
oh I see you are getting the index name too.
can we add java docs then.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java:
##########
@@ -61,4 +62,9 @@ public void register(HoodieTableMetaClient metaClient, String indexName, String
* Create a functional index.
*/
public abstract void create(HoodieTableMetaClient metaClient, String indexName, String indexType, Map<String, Map<String, String>> columns, Map<String, String> options);
+
+ /**
+ * Drop an index. By default, ignore drop if index does not exist.
+ */
+ public abstract void drop(HoodieTableMetaClient metaClient, String indexName, MetadataPartitionType metadataPartitionType, boolean ignoreIfNotExists);
Review Comment:
isn't the 3rd arg is needs fixes?
for sec and functional, the partition type could be same, but it could refer to diff indexes depending on the col we built the index on.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/index/functional/BaseHoodieIndexClient.java:
##########
@@ -61,4 +62,9 @@ public void register(HoodieTableMetaClient metaClient, String indexName, String
* Create a functional index.
*/
public abstract void create(HoodieTableMetaClient metaClient, String indexName, String indexType, Map<String, Map<String, String>> columns, Map<String, String> options);
+
+ /**
+ * Drop an index. By default, ignore drop if index does not exist.
+ */
+ public abstract void drop(HoodieTableMetaClient metaClient, String indexName, MetadataPartitionType metadataPartitionType, boolean ignoreIfNotExists);
Review Comment:
also, can we compute the metadataPartitionType internally.
```
val metadataPartitionType = MetadataPartitionType.fromPartitionPath(indexName)
```
we don't need the 3rd arg then.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [HUDI-7563] Add support to drop index using sql [hudi]
Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on code in PR #11951:
URL: https://github.com/apache/hudi/pull/11951#discussion_r1772378834
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/IndexCommands.scala:
##########
@@ -76,7 +76,14 @@ case class DropIndexCommand(table: CatalogTable,
override def run(sparkSession: SparkSession): Seq[Row] = {
val tableId = table.identifier
val metaClient = createHoodieTableMetaClient(tableId, sparkSession)
- SecondaryIndexManager.getInstance().drop(metaClient, indexName, ignoreIfNotExists)
+ try {
+ // need to ensure that the index name is for a valid partition type
+ MetadataPartitionType.fromPartitionPath(indexName)
+ HoodieSparkIndexClient.getInstance(sparkSession).drop(metaClient, indexName, ignoreIfNotExists)
+ } catch {
+ case _: IllegalArgumentException =>
+ SecondaryIndexManager.getInstance().drop(metaClient, indexName, ignoreIfNotExists)
Review Comment:
Got it. Let's have JIRA to track that.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org