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

map-string-string dimension column contrib extension #10628

Closed
wants to merge 1 commit into from

Conversation

himanshug
Copy link
Contributor

Description

Introduces a "mapStringString" complex type dimension column. All the code lives in an isolated contrib extension and implements relevant extensible Druid core interfaces e.g. ComplexColumn, ComplexMetricSerde, DimensionHandler, DimensionIndexer, DimensionMerger etc etc.

It allows ingestion of Map<String,String> column in users input data as a single dimension column . At query time, user can access individual keys in the map as if they were simple string dimension columns (see MapStringStringKeyVirtualColumn.java ) or column can be accessed as records of Map<String,String> type but that does not have much use in Druid query layer for now aside from select query.

User would have to implement their own InputRowParser to supply Map<String,String> records for ingestion , we used one that was very specific to our needs.

This extension also serves as an example of how users can create their own dimension columns for the specific needs and also [indirectly] tests the dimension extensibility support introduced in #10277

Future: Now that VirtualColumn interface has been enhanced to support vectorization, maybe the virtual column implementation code in this extension could also be improved to support that for [potentially] better performance.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@lgtm-com
Copy link

lgtm-com bot commented Dec 4, 2020

This pull request introduces 2 alerts when merging f306b31 into ae6c43d - view on LGTM.com

new alerts:

  • 1 for Result of multiplication cast to wider type
  • 1 for Potential input resource leak

@stale
Copy link

stale bot commented Apr 30, 2022

This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the dev@druid.apache.org list. Thank you for your contributions.

@stale stale bot added the stale label Apr 30, 2022
@a2l007 a2l007 removed the stale label Jun 1, 2022
@github-actions
Copy link

github-actions bot commented Oct 4, 2023

This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the dev@druid.apache.org list.
Thank you for your contributions.

@github-actions github-actions bot added the stale label Oct 4, 2023
Copy link

github-actions bot commented Nov 1, 2023

This pull request/issue has been closed due to lack of activity. If you think that
is incorrect, or the pull request requires review, you can revive the PR at any time.

@github-actions github-actions bot closed this Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants