Skip to content

Link up row-based datasources to serving layer.#9503

Merged
gianm merged 2 commits intoapache:masterfrom
gianm:joins-linkup-rowbased-to-server
Mar 11, 2020
Merged

Link up row-based datasources to serving layer.#9503
gianm merged 2 commits intoapache:masterfrom
gianm:joins-linkup-rowbased-to-server

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Mar 11, 2020

  • Add SegmentWrangler interface that allows linking of DataSources to Segments.
  • Add LocalQuerySegmentWalker that uses SegmentWranglers to compute queries on
    data that is available locally.
  • Modify ClientQuerySegmentWalker to use LocalQuerySegmentWalker when the base
    datasource is concrete and not a table.
  • Add SegmentWranglerModule to the Broker so it has them available and can
    properly instantiate . LocalQuerySegmentWalkers.
  • Set InlineDataSource and LookupDataSource to concrete, since they can be
    directly queried now.

- Add SegmentWrangler interface that allows linking of DataSources to Segments.
- Add LocalQuerySegmentWalker that uses SegmentWranglers to compute queries on
  data that is available locally.
- Modify ClientQuerySegmentWalker to use LocalQuerySegmentWalker when the base
  datasource is concrete and not a table.
- Add SegmentWranglerModule to the Broker so it has them available and can
  properly instantiate . LocalQuerySegmentWalkers.
- Set InlineDataSource and LookupDataSource to concrete, since they can be
  directly queried now.
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.

rad, this lgtm 🤘

It might be nice to have some additional test coverage with other query engines to make sure there isn't any funny stuff, but since this functionality isn't really documented yet I'm ok if this is filled out in one or more future PRs.

Also, there seems to be a legitimate test failure, you forgot to update InlineDataSourceTest.test_isConcrete to reflect the newly concrete status of InlineDataSource.

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Mar 11, 2020

It might be nice to have some additional test coverage with other query engines to make sure there isn't any funny stuff, but since this functionality isn't really documented yet I'm ok if this is filled out in one or more future PRs.

There should be a lot more in a future patch that connects this to the SQL layer. But for now, in this patch, there are integration tests.

Also, there seems to be a legitimate test failure, you forgot to update InlineDataSourceTest.test_isConcrete to reflect the newly concrete status of InlineDataSource.

Thanks, I'll fix that. I also got one of the integration tests wrong, which I'll fix:

 expectedResults: [{"version":"v1","timestamp":"0000-01-01T00:00:00.000Z","event":{"k":"Wikipedia:Vandalismusmeldung","v":"lookup!","nonexistent":null,"rows":1}}] 

 actualResults : [{"version":"v1","timestamp":"0000-01-01T00:00:00.000Z","event":{"v":"inline join!","k":"Wikipedia:Vandalismusmeldung","rows":1,"nonexistent":null}}]

@gianm gianm merged commit 2ef5c17 into apache:master Mar 11, 2020
@gianm gianm deleted the joins-linkup-rowbased-to-server branch March 11, 2020 18:32
@jihoonson jihoonson added this to the 0.18.0 milestone Mar 26, 2020
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.

3 participants