Skip to content

remove native scan query legacy mode#16659

Merged
clintropolis merged 7 commits intoapache:masterfrom
clintropolis:remove-scan-legacy-mode
Jul 19, 2024
Merged

remove native scan query legacy mode#16659
clintropolis merged 7 commits intoapache:masterfrom
clintropolis:remove-scan-legacy-mode

Conversation

@clintropolis
Copy link
Copy Markdown
Member

Description

The native scan query was moved from a contrib extension into core in Druid 0.11 in #4751, which introduced a 'legacy' option to maintain backwards compatibility with the extension, which used a non-standard time column name and always included this timestamp in the results.

I think sufficient time has passed to allow us to remove this parameter to help simplify code a bit.

Release note

The native scan query 'legacy' mode has been removed, which was introduced in Druid 0.11 to maintain compatibility during an upgrade from older versions of Druid where the scan query was part of a 'contrib' extension.


This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • been tested in a test Druid cluster.

@clintropolis clintropolis force-pushed the remove-scan-legacy-mode branch from ea3a7cf to 670bc02 Compare June 26, 2024 08:39
@clintropolis
Copy link
Copy Markdown
Member Author

CI failures are related to coverage, which I guess means this stuff wasn't well covered in the first place since I am only removing stuff in this PR. Can look into maybe adding some more tests to get it happy, but also doesn't really feel like a blocker

}

@Nullable
public RowSignature getRowSignature(boolean defaultIsLegacy)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

great to see this go! :)

@kgyrtkirk
Copy link
Copy Markdown
Member

kgyrtkirk commented Jun 27, 2024

oh...covereage change issues have prevented further tests from being run...
or somehow re-organize the maven target order...
I think there should be a way to disable that; this is a perfect example - there were issues arising from that earlier

In this case I think that doesn't really apply as these jobs were running with -pl processing already - but coverage issues are evil when they happen in the other modules splits


## Legacy mode

In older versions of Druid, the scan query supported a legacy mode designed for protocol compatibility with the former scan-query contrib extension from versions of Druid older than 0.11. This mode has been removed.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: Should we also add that this mode is removed from druid 31.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i considered this, but then also thought it was kind of embarrassing that it took so long to remove it, so not really sure we needed to highlight that. Also I can't imagine that many people are using it, i think the only reason to do it is if you were originally using the contrib extension. The only reason I didn't remove it entirely from the docs was just in case people are searching for what it means in native query json stuff have something to reference.

If you think there is some actionable purpose to specify which version it was removed in i can add it, but it doesn't seem too important to me.

@clintropolis clintropolis merged commit 35b8764 into apache:master Jul 19, 2024
@clintropolis clintropolis deleted the remove-scan-legacy-mode branch July 19, 2024 06:33
edgar2020 pushed a commit to edgar2020/druid that referenced this pull request Jul 19, 2024
edgar2020 pushed a commit to edgar2020/druid that referenced this pull request Jul 19, 2024
@gianm gianm added this to the 31.0.0 milestone Jul 24, 2024
abhishekagarwal87 pushed a commit that referenced this pull request Jul 25, 2024
#16793)

Fixes rolling downgrades/upgrades after #16659 by hard coding scan query "legacy":false since it is a required property during deserialization.
sreemanamala pushed a commit to sreemanamala/druid that referenced this pull request Aug 6, 2024
sreemanamala pushed a commit to sreemanamala/druid that referenced this pull request Aug 6, 2024
apache#16793)

Fixes rolling downgrades/upgrades after apache#16659 by hard coding scan query "legacy":false since it is a required property during deserialization.
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.

5 participants