Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix map get return double nested nullable #15230

Merged
merged 1 commit into from
Apr 14, 2024

Conversation

ariesdevil
Copy link
Member

@ariesdevil ariesdevil commented Apr 13, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

for this query:

CREATE OR REPLACE TABLE t1(tags Map(String, String)) Engine = Fuse

statement ok
INSERT INTO t1 VALUES ({'region':'sg', 'az': '1'}),({'region':'sg', 'az': '2'}), ({'region':'hk', 'az': '1'}),({'region':'hk', 'az': '2'})

query I
SELECT to_int32(tags['az']) AS int_az FROM t1 WHERE tags['region'] = 'sg' ORDER BY int_az;

The tags[az] will call map#get with input data type MapType<GenericType<0>, GenericType<1>> and return data type NullableType<GenericType<1>>, however, if GenericType<1> is already a nullable type, this will return Nullable<Nullable<XXX>>.

When this return type pass to to_int32, the try_downcast_column().unwrap() will failed.

This PR add a new get function for map get that catch nullable value in map first.

  • Fixes #[Link the issue here]

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe):

This change is Reviewable

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Apr 13, 2024
Copy link

what-the-diff bot commented Apr 13, 2024

PR Summary

  • Introduction of Updated "Get" Function
    This update includes a new version of the "get" function that can handle maps with values that may or may not exist (nullable values). This allows more flexibility when working with these types of data structures.

  • Inclusion of Revised "Get" Function in Function List
    The new "get" function has also been incorporated into the existing list of available functions, making it conveniently available for use in future coding tasks.

  • Modification of "Get" Function Test Cases
    The test cases specifically designed for the "get" function have been revised so they now use this new implementation. This ensures the updated function operates as expected.

  • Addition of Test Cases for Operation on Table with Map Column
    New test cases have been introduced to check operations like inserting and querying on tables having a column of the map types. By ensuring diverse test environment, we are increasing the robustness and reliability of our application.

  • Corrections in Map Column Table Creation Test Case
    An error identified in a test case that handles the creation of tables with a map column has been fixed, increasing overall testing accuracy and smoothness of operation in this scenario.

@ariesdevil
Copy link
Member Author

The failed unit test may not related this PR, seems a flaky test:

Summary [ 190.527s] 1614 tests run: 1613 passed (2 slow), 1 failed, 9 skipped
        FAIL [   0.106s] databend-storages-common-index::it filters::bloom_filter::test_bloom_filter

@ariesdevil
Copy link
Member Author

The failed unit test may not related this PR, seems a flaky test:

Summary [ 190.527s] 1614 tests run: 1613 passed (2 slow), 1 failed, 9 skipped
        FAIL [   0.106s] databend-storages-common-index::it filters::bloom_filter::test_bloom_filter

This actually related to this PR, fixed.

@BohuTANG BohuTANG merged commit 13afda7 into datafuselabs:main Apr 14, 2024
72 checks passed
@ariesdevil ariesdevil deleted the map_get branch April 14, 2024 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants