[FLINK-14301][doc] add documentation for functions categories and new function resolution orders #10110

bowenli86 · 2019-11-06T23:30:45Z

What is the purpose of the change

unifies function related docs in a single dir/tab
add general introduction of function categories, reference mechanism, and resolution orders

Brief change log

renamed functions.md to builtinFunctions.md
unifies function related docs in a single dir/tab of docs/dev/table/functions/
add general introduction of function categories, reference mechanism, and resolution orders

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

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: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? (no)
If yes, how is the feature documented? (docs)

bowenli86 · 2019-11-06T23:31:42Z

@sjwiesman @xuefuz @lirui-apache @zjuwangg

flinkbot · 2019-11-06T23:34:01Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit c27cac4 (Wed Dec 04 15:07:08 UTC 2019)

✅no warnings

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❗ 3. Needs [attention] from.
- Needs attention by @dawidwys [committer], @fhueske [PMC], @sjwiesman
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

flinkbot · 2019-11-07T00:08:26Z

CI report:

4295284 : SUCCESS Build
1c59dc7 : SUCCESS Build
c27cac4 : SUCCESS Build

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run travis re-run the last Travis build

bowenli86 · 2019-11-07T00:34:06Z

docs of how to register and manipulate such functions will be added later, as ddl and apis are still moving around and not finalized yet

dawidwys · 2019-11-07T09:06:57Z

@flinkbot attention @fhueske @sjwiesman

sjwiesman

I've made a first pass of the docs.

Besides my inline comments, please change the parent nav on the pages you moved to be 'table_functions'.

sjwiesman · 2019-11-07T15:47:47Z

docs/dev/table/functions/index.md

+nav-id: table_functions
+nav-parent_id: tableapi
+nav-pos: 116
+is_beta: true


Why are functions beta?

sjwiesman · 2019-11-07T16:44:25Z

docs/dev/table/functions/index.md

+
+There are two dimensions to classify functions in Flink.
+
+One dimension is system (or built-in) functions v.s. catalog functions. System functions have no namespace and can be


You say system here but named the page built in. Let's just pick one and stick with it.

let's use "system"

sjwiesman · 2019-11-07T16:46:15Z

docs/dev/table/functions/index.md

+Types of Functions
+------------------
+
+There are two dimensions to classify functions in Flink.
+
+One dimension is system (or built-in) functions v.s. catalog functions. System functions have no namespace and can be
+referenced with just their names. Catalog functions belong to a catalog and database therefore they have catalog and database
+namespaces, they can be referenced by either fully/partially qualified name (`catalog.db.func` or `db.func`) or just the
+function name.
+
+The other dimension is temporary functions v.s. persistent functions. Temporary functions are volatile and only live up to
+ lifespan of a session, they are always created by users. Persistent functions live across lifespan of sessions, they are either
+ provided by the system or persisted in catalogs.
+
+The two dimensions give Flink users 4 categories of functions:
+
+1. Temporary system functions
+2. System functions
+3. Temporary catalog functions
+4. Catalog functions
+


This is conflating properties that I don't feel are related. built-in vs catalog and temporary vs persistent should be seperate sections.

I actually feel they are related to help users understand resolution order below. Maybe I should have mentioned that in resolution order, system functions always precede catalog's and temporary functions always precede persistent on its own dimension

sjwiesman · 2019-11-07T16:46:31Z

docs/dev/table/functions/index.md

+The two dimensions give Flink users 4 categories of functions:
+
+1. Temporary system functions
+2. System functions
+3. Temporary catalog functions
+4. Catalog functions


I think this is unnecessary and can be removed.

sjwiesman · 2019-11-07T16:46:37Z

docs/dev/table/functions/index.md

+-->
+
+Flink Table API & SQL empowers users to do data transformations with functions.
+


sjwiesman · 2019-11-07T16:47:16Z

docs/dev/table/functions/index.md

+
+In ambiguous function reference, users just specify the function's name in SQL query, e.g. `select myfunc(x) from mytable`.
+
+This is the only supported option before Flink 1.10.


Remove, if a user is running an old version of Flink they can reference that versions docs.

sjwiesman · 2019-11-07T16:49:00Z

docs/dev/table/functions/index.md

+In Flink 1.10, we also redefined its function resolution order to be:
+
+The new order is:


No need to qualify, if a user is running an old version of Flink they can reference that versions docs.

the old version actually doesn't talk about this. and this will be a disruptive change

I don't understand this change well enough to be able to respond to that, however, I generally feel these kinds of changes are what release notes are for. I'd like to hear @dawidwys or @fhueske opinion and if they say it stays then +1 from me.

I agree with @sjwiesman. Personally I think the documentation should just describe the current state. Unless this is a disruptive change that requires users to change their code. I don't think this is the case. I

f I am wrong, and it does, I would mention the version, but also would mention what are the actions that a user must take and would change the title to something along the lines: "Migration guide/Migration"

sjwiesman · 2019-11-07T16:51:51Z

docs/dev/table/functions/index.md

+title: "Functions"
+nav-id: table_functions
+nav-parent_id: tableapi
+nav-pos: 116


This renders very low down, I think functions should be higher up on the side nav.

bowenli86 · 2019-11-07T22:38:41Z

@sjwiesman thanks for your review. pls take another look

bowenli86 · 2019-11-11T21:29:12Z

@flinkbot attention @sjwiesman @dawidwys @fhueske

dawidwys · 2019-11-12T16:52:03Z

Sorry I missed the notifications. I have no major concerns regarding the content. However I would prefer somebody better with the linguistic aspect gave the final approval.

…ed dir /dev/table/functions

… function resolution orders

bowenli86 · 2019-11-12T20:16:54Z

@sjwiesman I removed the 1.10 info, please take another look

sjwiesman · 2019-11-12T20:28:16Z

+1 to merge

rmetzger added review=description? component=TableSQL/API labels Nov 6, 2019

zjuwangg approved these changes Nov 7, 2019

View reviewed changes

sjwiesman requested changes Nov 7, 2019

View reviewed changes

bowenli86 added 4 commits November 12, 2019 12:14

[FLINK-14638][doc] move functions related docs to a new single, unifi…

5cdaa58

…ed dir /dev/table/functions

[FLINK-14301][doc] add documentation for functions categories and new…

6c26a54

… function resolution orders

address feedbacks

9d5ed57

address feedback

c27cac4

bowenli86 closed this in a20eb65 Nov 12, 2019


		There are two dimensions to classify functions in Flink.

		One dimension is system (or built-in) functions v.s. catalog functions. System functions have no namespace and can be

		-->

		Flink Table API & SQL empowers users to do data transformations with functions.


		In ambiguous function reference, users just specify the function's name in SQL query, e.g. `select myfunc(x) from mytable`.

		This is the only supported option before Flink 1.10.

		In Flink 1.10, we also redefined its function resolution order to be:

		The new order is:

[FLINK-14301][doc] add documentation for functions categories and new function resolution orders #10110

[FLINK-14301][doc] add documentation for functions categories and new function resolution orders #10110

Uh oh!

Conversation

bowenli86 commented Nov 6, 2019

What is the purpose of the change

Brief change log

Verifying this change

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

Documentation

Uh oh!

bowenli86 commented Nov 6, 2019

Uh oh!

flinkbot commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automated Checks

Review Progress

Uh oh!

flinkbot commented Nov 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

bowenli86 commented Nov 7, 2019

Uh oh!

dawidwys commented Nov 7, 2019

Uh oh!

sjwiesman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bowenli86 commented Nov 7, 2019

Uh oh!

bowenli86 commented Nov 11, 2019

Uh oh!

dawidwys commented Nov 12, 2019

Uh oh!

bowenli86 commented Nov 12, 2019

Uh oh!

sjwiesman commented Nov 12, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

flinkbot commented Nov 6, 2019 •

edited

Loading

flinkbot commented Nov 7, 2019 •

edited

Loading