PHOENIX-7008 Implementation for CREATE CDC #1681

haridsv · 2023-09-20T13:25:54Z

This PR builds on top of the shallow grammar support added for CREATE CDC grammar as part of #1662. This change makes the required syscat changes along with the creation of the underly index. There are quite a few TODO items that need to be discussed, but this PR can still be merged as we can address them in future PRs.

Jira: PHOENIX-7008

… underlying hbase table

…ENIX-7001_2

virajjasani · 2023-09-26T22:00:54Z

bin/sqlline.py

@@ -114,6 +116,7 @@ def kill_child():
    disable_jna = ""

 java_cmd = java + ' $PHOENIX_OPTS ' + \
+    (args.debug and "-Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=127.0.0.1:8071" or "") + \


this is local change right?

I am hoping to include this as it is quite useful. Do you see any problem?

virajjasani · 2023-09-26T22:01:10Z

bin/sqlline.py

-    " -u jdbc:phoenix:" + phoenix_utils.shell_quote([zookeeper]) + \
-    " -n none -p none --color=" + colorSetting + " --fastConnect=" + tryDecode(args.fastconnect) + \
-    " --verbose=" + tryDecode(args.verbose) + " --incremental=false --isolation=TRANSACTION_READ_COMMITTED " + sqlfile
+    (not args.noconnect and " -u jdbc:phoenix:" + phoenix_utils.shell_quote([zookeeper]) or "") + \


same here, all changes in this file can be removed

I would like to include the changes in the sqlline, unless there is a problem with the approach. I can also raise it as a separate PR if that is preferred.

yeah i think separate Jira would be better to track it and others can also review, i am not actually sure whether we can make the change or not because at least we don't use this script as is in production env

I now moved the change into PHOENIX-7055 and created a separate PR #1695. Please take a look.

virajjasani · 2023-09-26T22:02:47Z

phoenix-core/src/it/java/org/apache/phoenix/end2end/CDCMiscIT.java

+public class CDCMiscIT extends ParallelStatsDisabledIT {
+    private void assertCDCState(Connection conn, String cdcName, String expInclude,
+                                int idxType) throws SQLException {
+        try (ResultSet rs = conn.createStatement().executeQuery("SELECT cdc_include FROM " +


Once all the SELECT queries to SYSTEM.CATALOG are replaced with PTable based comparison, we are good to go

I have added the check for the change scope on PTable as it corresponds to the protobuf change but left the existing SYSCAT checks as is, as these are existing fields and don't necessarily have to be validated through PTable. WDYT?

… not parse

virajjasani · 2023-09-28T01:00:16Z

Looks like PTableType change from previous commit caused the conflict

virajjasani · 2023-09-28T01:08:53Z

looks good otherwise

…ENIX-7001_2

haridsv · 2023-09-28T11:39:04Z

FIxed the conflict and moved out the sqlline.py change.

virajjasani · 2023-09-28T20:44:18Z

Some test failures reported with error:

ERROR 504 (42703): Undefined column. columnName=SYSTEM.CATALOG.CDC_INCLUDE

https://ci-hadoop.apache.org/job/Phoenix/job/Phoenix-PreCommit-GitHub-PR/job/PR-1681/5/testReport/

otherwise, we are good to go.

lokiore · 2023-09-28T21:48:26Z

phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java

@@ -4107,6 +4109,9 @@ protected PhoenixConnection upgradeSystemCatalogIfRequired(PhoenixConnection met
            metaConnection = addColumnsIfNotExists(metaConnection, PhoenixDatabaseMetaData.SYSTEM_CATALOG,
                MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0,


Here MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 should be increased by 1 and then each addColumns above will decrease it by 1. This can lead to BackwardCompatibilityIT tests to fail, which we are seeing,

Done, thank you!

The tests are still failing for the same reason, but this time on the PHYSICAL_TABLE_NAME column (the very first one). Could you please check to see what I might have done wrong?

Value of MIN_SYSTEM_TABLE_TIMESTAMP needs to be increased by 1 as well here :-

phoenix/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataProtocol.java

Line 102 in 6125f02

public static final long MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 = MIN_TABLE_TIMESTAMP + 37;

Address the changes in the CREATE CDC spec in PHOENIX-7001 Also includes some review feedback changes across the changes in PRs apache#1681, apache#1703 and apache#1766

This commit includes a squash of all the below changes from the PRs apache#1662, apache#1694, down by removing non-functional changes for the ease of review. On top of it, the changes have been reworked for the changed state of master, especially the new submodules. * 4c9827a Hari.. PHOENIX-7008 Shallow grammar support for CREATE CDC (apache#1662) * e5220e0 TheN.. PHOENIX-7054 Shallow grammar support for DROP CDC and ALTER CDC (apache#1694) * 581e613 Hari.. PHOENIX-7008 Implementation for CREATE CDC (apache#1681) * e2ef886 Hari.. PHOENIX-7008 Tweaks, fixes and additional test coverage for CREATE CDC (apache#1703) * 5d3fd40 TheN.. PHOENIX-7074 DROP CDC Implementation (apache#1713) * 7420443 Hari.. PHOENIX-7014: Query compiler/optimizer changes along with some PHOENIX-7015 changes (apache#1766) * da6ddad Kadi.. Add an extra delete mutation for CDC * 93d586e Kadi.. Add an extra delete mutation during rebuild for CDC index * f07898f Hari.. PHOENIX-7008: Addressing Jira spec and review feedback changes (apache#1802) * 87a2ea1 Hari.. PHOENIX-7008: Fix for parser gap and fix for failing test (apache#1812) * e395780 TheN.. PHOENIX-7015 Implementing CDCGlobalIndexRegionScanner (apache#1813) Co-authored-by: Saurabh Rai <saurabh.rai@salesforce.com>

…ture This commit includes a squash of all the below changes from the PRs apache#1662, apache#1694, down by removing non-functional changes for the ease of review. On top of it, the changes have been reworked for the changed state of master, especially the new submodules. * 4c9827a Hari.. PHOENIX-7008 Shallow grammar support for CREATE CDC (apache#1662) * e5220e0 TheN.. PHOENIX-7054 Shallow grammar support for DROP CDC and ALTER CDC (apache#1694) * 581e613 Hari.. PHOENIX-7008 Implementation for CREATE CDC (apache#1681) * e2ef886 Hari.. PHOENIX-7008 Tweaks, fixes and additional test coverage for CREATE CDC (apache#1703) * 5d3fd40 TheN.. PHOENIX-7074 DROP CDC Implementation (apache#1713) * 7420443 Hari.. PHOENIX-7014: Query compiler/optimizer changes along with some PHOENIX-7015 changes (apache#1766) * da6ddad Kadi.. Add an extra delete mutation for CDC * 93d586e Kadi.. Add an extra delete mutation during rebuild for CDC index * f07898f Hari.. PHOENIX-7008: Addressing Jira spec and review feedback changes (apache#1802) * 87a2ea1 Hari.. PHOENIX-7008: Fix for parser gap and fix for failing test (apache#1812) * e395780 TheN.. PHOENIX-7015 Implementing CDCGlobalIndexRegionScanner (apache#1813) Co-authored-by: Saurabh Rai <saurabh.rai@salesforce.com>

haridsv added 11 commits September 19, 2023 20:48

Fill in the code needed to create underlying index

720efd0

Crude implementation of CDC table creation, need to avoid creation of…

a61087a

… underlying hbase table

Do not create physical hbase table and other tweaks

5e97171

Some ITs and fix a gap

1ff2b48

CDC scope set/string conversion

0b30f87

More test coverage and a fix for index type

ac4a869

Merge remote-tracking branch 'upstream/PHOENIX-7001-feature' into PHO…

72d55ad

…ENIX-7001_2

javadoc

a464166

Ensure resources are closed, fix SpotBugs warnings

967b56d

Fix failing test

281c703

Minor refactoring and util

24cda1d

virajjasani self-requested a review September 26, 2023 16:00

virajjasani reviewed Sep 26, 2023

View reviewed changes

haridsv added 3 commits September 27, 2023 11:38

Add assertion on the PTable to validate protobuf changes

712cb60

Recognize INDEX_TYPE=l for local index as local is a keyword and will…

f07391b

… not parse

Translate the index already exists exception

37d0518

haridsv added 2 commits September 28, 2023 17:05

Moved the change into PHOENIX-7055

442a448

Merge remote-tracking branch 'upstream/PHOENIX-7001-feature' into PHO…

f8c81fd

…ENIX-7001_2

haridsv requested a review from virajjasani September 28, 2023 11:39

FIll in the implementation for a dummy method

d8b6145

lokiore reviewed Sep 28, 2023

View reviewed changes

haridsv added 5 commits September 29, 2023 11:11

Fix failing BackwardCompatibilityIT test and some line reformatting

12b206d

Simply scope enums and add tests

9e9e31b

Tweak the change scope handling when not specified

aae9c3e

Fix a miss in if not exists handling

e99d103

Also bump up MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0

3fe4fde

virajjasani approved these changes Sep 30, 2023

View reviewed changes

virajjasani merged commit 581e613 into apache:PHOENIX-7001-feature Sep 30, 2023
0 of 5 checks passed

haridsv mentioned this pull request Oct 6, 2023

PHOENIX-7008 Tweaks, fixes and additional test coverage for CREATE CDC #1703

Merged

haridsv mentioned this pull request Jan 22, 2024

PHOENIX-7008: Addressing Jira spec and review feedback changes #1802

Merged

haridsv mentioned this pull request Mar 29, 2024

PHOENIX-7001: Initial implementation of Change Data Capture (CDC) feature #1866

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PHOENIX-7008 Implementation for CREATE CDC #1681

PHOENIX-7008 Implementation for CREATE CDC #1681

haridsv commented Sep 20, 2023

virajjasani Sep 26, 2023

haridsv Sep 27, 2023

virajjasani Sep 26, 2023

haridsv Sep 27, 2023

virajjasani Sep 28, 2023

haridsv Sep 28, 2023

virajjasani Sep 26, 2023

haridsv Sep 27, 2023

virajjasani commented Sep 28, 2023

virajjasani commented Sep 28, 2023

haridsv commented Sep 28, 2023

virajjasani commented Sep 28, 2023 •

edited

lokiore Sep 28, 2023

haridsv Sep 29, 2023

haridsv Sep 29, 2023

lokiore Sep 29, 2023

		@@ -4107,6 +4109,9 @@ protected PhoenixConnection upgradeSystemCatalogIfRequired(PhoenixConnection met
		metaConnection = addColumnsIfNotExists(metaConnection, PhoenixDatabaseMetaData.SYSTEM_CATALOG,
		MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0,

PHOENIX-7008 Implementation for CREATE CDC #1681

PHOENIX-7008 Implementation for CREATE CDC #1681

Conversation

haridsv commented Sep 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

virajjasani commented Sep 28, 2023

virajjasani commented Sep 28, 2023

haridsv commented Sep 28, 2023

virajjasani commented Sep 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

virajjasani commented Sep 28, 2023 •

edited