Skip to content

Conversation

@EronWright
Copy link
Contributor

What is the purpose of the change

Based on #7392. Please see commits prefixed with FLINK-9172.

Adds support for catalogs to the SQL Client environment file, using descriptors. For example, here's a sample catalog of type taxi-data being registered into sql-client-defaults.yaml:

catalogs:
  - name: nyc
    type: taxi-data
    rides-file: /Users/david/stuff/flink-training/trainingData/nycTaxiRides.gz
    fares-file: /Users/david/stuff/flink-training/trainingData/nycTaxiFares.gz

Brief change log

[FLINK-9172] [table] [sql-client] - Support external catalogs in SQL-Client

  • add CatalogEntry to configuration types
  • enhance Environment to include catalogs in overall configuration
  • enhance ExecutionContext to register the configured catalogs
  • cleanup test-table-factories.yaml (references to non-existent classes)
  • update test dependencies of sql-client to include flink-table's test classes (for TestExternalCatalogFactory)

Verifying this change

This change added tests and can be verified as follows:

  • enhanced unit test DependencyTest to test discovery of catalog factories
  • enhanced unit test EnvironmentTest to test merging of catalog configuration
  • enhanced unit test EnvironmentTest to test the detection of duplicate catalogs/tables/functions
  • enhanced unit test ExecutionContextTest to test external catalogs

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): no
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? yes
  • If yes, how is the feature documented? not documented (see FLINK-11245)

…y table

- fix the logic in supportsBatch to properly declare a batch-only table
- adjust CommonTestData to provide batch-only or streaming-only tables

Signed-off-by: EronWright <eronwright@gmail.com>
Signed-off-by: EronWright <eronwright@gmail.com>
…w/ user classloader

- use the context classloader for all interactions with TableEnvironment

Signed-off-by: EronWright <eronwright@gmail.com>
Signed-off-by: EronWright <eronwright@gmail.com>
- add is-streaming property for increased flexibility
- fix style issue

Signed-off-by: EronWright <eronwright@gmail.com>
…og via a descriptor

- add ConnectExternalCatalogDescriptor
- add ConnectExternalCatalogDescriptorTest
- extend TableEnvironment with new connect method for catalogs

Signed-off-by: EronWright <eronwright@gmail.com>
…og via a descriptor

- add `TableEnvironment::listExternalCatalogs`

Signed-off-by: EronWright <eronwright@gmail.com>
…og via a descriptor

- fix scalastyle check

Signed-off-by: EronWright <eronwright@gmail.com>
…Client

- add `CatalogEntry` to configuration types
- enhance `Environment` to include `catalogs` in overall configuration
- enhance `ExecutionContext` to register the configured catalogs
- cleanup test-table-factories.yaml (references to non-existent classes)
- enhance `DependencyTest` to test discovery of catalog factories
- enhance `EnvironmentTest` to test merging of catalog configuration
- enhance `EnvironmentTest` to test the detection of duplicate catalogs/tables/functions
- enhance `ExecutionContextTest` to test external catalogs
- update test dependencies of sql-client to include flink-table's test classes (for `TestExternalCatalogFactory`)

Signed-off-by: EronWright <eronwright@gmail.com>
@EronWright
Copy link
Contributor Author

@twalthr ping

@rmetzger
Copy link
Contributor

@twalthr @walterddr want's the status of this PR?

@twalthr
Copy link
Contributor

twalthr commented Feb 26, 2019

This is a valuable contribution and definitely a feature that we want to have. My only concern here is that this PR exposes a catalog interface at another end which will be deprecated very soon as part of FLINK-11275. I would suggest to wait until FLINK-11476 has been merged.

@twalthr
Copy link
Contributor

twalthr commented Mar 13, 2019

@EronWright it seems that the unified catalog API might be delayed and it makes sense to allow this feature sooner. I will review this pull request today.

@bowenli86
Copy link
Member

Thanks for your valuable contribution, @EronWright ! I left some comments in the JIRA ticket. Can you please rebase and adapt this PR? I can help review it as soon as you submit again. Thanks!

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.

4 participants