Skip to content

Fix lookup serde on node types that don't load lookups.#7752

Merged
fjy merged 1 commit intoapache:masterfrom
gianm:fix-lookup-serde-module-stuff
May 24, 2019
Merged

Fix lookup serde on node types that don't load lookups.#7752
fjy merged 1 commit intoapache:masterfrom
gianm:fix-lookup-serde-module-stuff

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented May 24, 2019

This includes the router, overlord, middleManager, and coordinator.
Does the following things:

  • Loads LookupSerdeModule on MM, overlord, and coordinator.
  • Adds LookupExprMacro to LookupSerdeModule, which allows these node
    types to understand that the 'lookup' function exists.
  • Adds a test to make sure that LookupSerdeModule works for virtual
    columns, filters, transforms, and dimension specs.

This is implementing the technique discussed on these two issues:

This includes the router, overlord, middleManager, and coordinator.
Does the following things:

- Loads LookupSerdeModule on MM, overlord, and coordinator.
- Adds LookupExprMacro to LookupSerdeModule, which allows these node
  types to understand that the 'lookup' function exists.
- Adds a test to make sure that LookupSerdeModule works for virtual
  columns, filters, transforms, and dimension specs.

This is implementing the technique discussed on these two issues:

- apache#7724 (comment)
- apache#7082 (comment)
@gianm gianm added the Bug label May 24, 2019
@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented May 24, 2019

This fixes a bug introduced in #7222 (the router used to know about the lookup function, but now it doesn't) so it's a regression in 0.15.0. It should be included.

@gianm gianm added this to the 0.15.0 milestone May 24, 2019
Copy link
Copy Markdown
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

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

lgtm 🤘

Copy link
Copy Markdown
Contributor

@jihoonson jihoonson left a comment

Choose a reason for hiding this comment

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

LGTM

@fjy fjy merged commit 7ec7257 into apache:master May 24, 2019
gianm added a commit to implydata/druid-public that referenced this pull request May 24, 2019
This includes the router, overlord, middleManager, and coordinator.
Does the following things:

- Loads LookupSerdeModule on MM, overlord, and coordinator.
- Adds LookupExprMacro to LookupSerdeModule, which allows these node
  types to understand that the 'lookup' function exists.
- Adds a test to make sure that LookupSerdeModule works for virtual
  columns, filters, transforms, and dimension specs.

This is implementing the technique discussed on these two issues:

- apache#7724 (comment)
- apache#7082 (comment)
@gianm gianm deleted the fix-lookup-serde-module-stuff branch May 24, 2019 20:29
gianm added a commit to gianm/druid that referenced this pull request May 27, 2019
This includes the router, overlord, middleManager, and coordinator.
Does the following things:

- Loads LookupSerdeModule on MM, overlord, and coordinator.
- Adds LookupExprMacro to LookupSerdeModule, which allows these node
  types to understand that the 'lookup' function exists.
- Adds a test to make sure that LookupSerdeModule works for virtual
  columns, filters, transforms, and dimension specs.

This is implementing the technique discussed on these two issues:

- apache#7724 (comment)
- apache#7082 (comment)
fjy pushed a commit that referenced this pull request May 27, 2019
This includes the router, overlord, middleManager, and coordinator.
Does the following things:

- Loads LookupSerdeModule on MM, overlord, and coordinator.
- Adds LookupExprMacro to LookupSerdeModule, which allows these node
  types to understand that the 'lookup' function exists.
- Adds a test to make sure that LookupSerdeModule works for virtual
  columns, filters, transforms, and dimension specs.

This is implementing the technique discussed on these two issues:

- #7724 (comment)
- #7082 (comment)
jihoonson pushed a commit to implydata/druid-public that referenced this pull request Jun 4, 2019
…apache#7767)

This includes the router, overlord, middleManager, and coordinator.
Does the following things:

- Loads LookupSerdeModule on MM, overlord, and coordinator.
- Adds LookupExprMacro to LookupSerdeModule, which allows these node
  types to understand that the 'lookup' function exists.
- Adds a test to make sure that LookupSerdeModule works for virtual
  columns, filters, transforms, and dimension specs.

This is implementing the technique discussed on these two issues:

- apache#7724 (comment)
- apache#7082 (comment)
jihoonson pushed a commit to implydata/druid-public that referenced this pull request Jun 26, 2019
This includes the router, overlord, middleManager, and coordinator.
Does the following things:

- Loads LookupSerdeModule on MM, overlord, and coordinator.
- Adds LookupExprMacro to LookupSerdeModule, which allows these node
  types to understand that the 'lookup' function exists.
- Adds a test to make sure that LookupSerdeModule works for virtual
  columns, filters, transforms, and dimension specs.

This is implementing the technique discussed on these two issues:

- apache#7724 (comment)
- apache#7082 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants