Skip to content

Conversation

@szucsvillo
Copy link
Contributor

…ype column in data table

@szucsvillo
Copy link
Contributor Author

Example:
create table test203 (mykey integer not null primary key, col1 bigint, new_column_1 FLOAT, new_column_2 DOUBLE, new_column_3 BIGINT);
create index indexName203 on test203 (col1) INCLUDE (new_column_1,new_column_2,new_column_3);
UPSERT INTO TEST203(mykey,col1,new_column_1,new_column_2,new_column_3) VALUES(5,43,201,202,203);
SELECT TENANT_ID, TABLE_SCHEM, TABLE_NAME, DATA_TYPE, COLUMN_NAME, DATA_TABLE_NAME, COLUMN_FAMILY,ORDINAL_POSITION, KEY_SEQ FROM system.catalog WHERE TABLE_NAME = 'INDEXNAME203' or TABLE_NAME = 'TEST203';

phoenix7282_goal

When creating an index like indexName203 on test203 (col1) with included columns (new_column_1, new_column_2, new_column_3), the data types of the included columns in the catalog table do not change to DECIMAL. This is because these columns are only included in the index for retrieval efficiency and are not part of the indexed key, like col1.
That's why I made the following changes in AlterAddCascadeIndexIT to ensure the included columns retain their original data types, such as FLOAT or DOUBLE, rather than converting them to DECIMAL, since they are not part of the index key and don't require transformation for indexing purposes.

@szucsvillo szucsvillo marked this pull request as draft October 15, 2024 07:06
@szucsvillo szucsvillo marked this pull request as ready for review October 16, 2024 07:30
Copy link
Contributor

@stoty stoty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM

@stoty stoty requested a review from gjacoby126 October 16, 2024 08:19
@stoty
Copy link
Contributor

stoty commented Oct 21, 2024

Please rebase @szucsvillo , this doesn't seem to have the fixes my BackwardsCompatibilityIT regression.

I'd like to see a clean(er) CI run before committing.

@stoty
Copy link
Contributor

stoty commented Oct 21, 2024

Kicked off another run, as the last one hasn't even made it into the ITs :(

@stoty stoty merged commit 9464c7f into apache:master Oct 21, 2024
asfgit pushed a commit that referenced this pull request Oct 21, 2024
asfgit pushed a commit that referenced this pull request Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants