Hi all, Thanks for driving this. It's very useful to have a framework around errors, definitely +1 on the direction!
One addition suggestion: can we keep the SQL position (start and end line/column) as structured data for parser and validator errors? It's best effort, since not every error has a position, but Calcite already computes it. Today it ends up only in the message text, and I've seen multiple clients, including us, resort to text parsing to extract it. Having it structured would let editors, clients and AI tools point at the exact fragment for planning errors. What do you think? Kind regards, Gustavo On Thu, 12 Nov 2026 at 09:55, Timo Walther via dev <[email protected]> wrote: > Thanks Sergey, if there are no objections, I would start a vote tomorrow. > > Thanks, > Timo > > > On 11.11.26 11:09, Sergey Nuyanzin wrote: > > Thank you for clarifying this Timo > > sounds good to me > > > > On Wed, Nov 11, 2026 at 10:09 AM Timo Walther via dev > > <[email protected]> wrote: > >> > >> Seems I forgot the some bullet points in my answer: > >> > >> 3. looks like FlinkPlannerImpl.validate does not catch AssertionError > >> > >> That's what I mentioned in my answer to 2. already, we can catch and > >> rewrite exceptions at critical locations. Esp in Planner and Parser > methods. > >> > >> > >> 4. Since FlinkError is an arg for FlinkRuntimeException, should it be > >> Serializeble? > >> > >> Yes, FlinkError should be Serializable. In general enums are > >> Serializable in Java, but I will add it explicitly to the FLIP. > >> > >> 5. There are examples of redacting messages from exception, however in > >> case of Calcite sensitive data will be also in exception trace, will > >> it be redacted anyhow? > >> > >> Section "Logging without user data" shows an example of how a redacted > >> message looks like. If our utils are good enough, we can also redact > >> sensitive data in stack traces. Calcite provides templates such as: > >> > >> @BaseMessage("Invalid number of arguments to function ''{0}''. Was > >> expecting {1,number,#} arguments") > >> ExInst<SqlValidatorException> invalidArgCount(String a0, int a1); > >> > >> Based on those we can classify which parts are sensitive whenever > >> Calcite adds a new method in CalciteResource. > >> > >> Cheers, > >> Timo > >> > >> > >> > >> > >> > >> On 09.11.26 14:53, Timo Walther via dev wrote: > >>> Hi Sergey, > >>> > >>> 1. what is INTERNAL ErrorCategory? > >>> > >>> INTERNAL is similar to IllegalStateException or > >>> UnsupportedOperationException; an error that should not reach the user. > >>> In case of SQL, a condition that is required to be met in order for the > >>> planner to continue. E.g. a switch case that cannot deal with the > >>> current input in a rule. As part of the implementation we will > >>> definitely add examples to the JavaDocs and make sure that each > category > >>> has clear explanation and guidelines. > >>> > >>> 2. there might be just assertionError > >>> > >>> We have to decide on a case by case how we want to deal with those. I > >>> could imagine wrapping some main entrypoints in the stack e.g. > >>> Parser.parse or Planner.compilePlan with a try-catch for > FlinkThrowable. > >>> In any case, the new error infra should make it possible to quickly > >>> identify whether this is known Flink error or a Calcite error that > needs > >>> to be classified and potentially rewritten into a FlinkError. > >>> > >>> In general, as I said before, this FLIP only provides a skeleton which > >>> we can use to eliminate also AssertionErrors, but we might still > >>> implement utils to catch exceptions that we cannot translate coming > from > >>> Calcite or other libs. > >>> > >>> Cheers, > >>> Timo > >>> > >>> On 09.11.26 14:26, Sergey Nuyanzin wrote: > >>>> Hi Timo > >>>> thank you for driving this very useful FLIP > >>>> > >>>> I have several clarifying questions > >>>> 1. what is INTERNAL ErrorCategory? > >>>> For instance an exception which came from Calcite is INTERNAL or > >>>> something else? > >>>> > >>>> 2. In the doc it is mentioned that > >>>>> Flink doesn't change throw sites in Calcite code, including its > >>>>> copies of Calcite classes, so Calcite upgrades stay simple. Instead, > >>>>> FlinkCalciteSqlValidator overrides newValidationError(SqlNode, > >>>>> ExInst), which most validator errors go through. The new > >>>>>> implementation becomes an enum mapping: > >>>> Besides newValidationError there might be just assertionError > >>>> (probably history reasons), simple example to repro: SELECT > >>>> nested_table['non_existed_field'] FROM nested_table > >>>> since newValidationError are mentioned in FLIP, should we mention > >>>> others or just talk about errors in general? > >>>> > >>>> 3. Also related to previous point(just an observation): not sure how > >>>> it will be in implementation however it looks like > >>>> FlinkPlannerImpl.validate does not catch AssertionError > >>>> 4. Since FlinkError is an arg for FlinkRuntimeException, should it be > >>>> Serializeble? > >>>> 5. There are examples of redacting messages from exception, however in > >>>> case of Calcite sensitive data will be also in exception trace, will > >>>> it be redacted anyhow? > >>>> > >>>> On Wed, Nov 4, 2026 at 4:00 PM Krishna Sai <[email protected]> > wrote: > >>>>> > >>>>> Hi Timo, > >>>>> > >>>>> That makes sense, and thanks for driving this. Infrastructure first > >>>>> is the > >>>>> right order, and a pass spanning releases sounds realistic for the > >>>>> scope. > >>>>> > >>>>> Looking forward to seeing the skeleton land. > >>>>> > >>>>> Best regards, > >>>>> Sepuri Sai Krishna > >>>>> > >>>>> On Wed, Nov 4, 2026 at 8:10 PM Timo Walther via dev > >>>>> <[email protected]> > >>>>> wrote: > >>>>> > >>>>>> Hi Krishna, > >>>>>> > >>>>>> just to set expectations right: This FLIP introduces the > infrastructure > >>>>>> for improving our error messages going forward. Once we come up > with a > >>>>>> solid skeleton, with the help of AI we will definitely make a pass > over > >>>>>> all existing exceptions. This includes CodeGenException. I don't > >>>>>> want to > >>>>>> give guarantees at this stage, because it is a large effort that > might > >>>>>> span multiple Flink releases. But the examples that you listed are > >>>>>> exactly the type of issues we want to tackle. > >>>>>> > >>>>>> Cheers, > >>>>>> Timo > >>>>>> > >>>>>> > >>>>>> On 04.11.26 14:51, Krishna Sai wrote: > >>>>>>> Hi Timo, > >>>>>>> > >>>>>>> This would help a lot, in particular putting the category on the > error > >>>>>>> rather than on the exception class. > >>>>>>> > >>>>>>> One measured example, if a second family is useful. In batch, a > MAP is > >>>>>>> rejected at every comparison site I tried, through three different > >>>>>>> exception classes: > >>>>>>> > >>>>>>> ORDER BY m and GROUP BY m both give ValidationException, "Type > >>>>>> 'MAP<STRING, > >>>>>>> INT> NOT NULL' cannot be ordered, so it cannot be used as a key for > >>>>>>> sorting, grouping, or joining". One template serves both, so a > >>>>>>> grouping > >>>>>>> failure reads as an ordering one. > >>>>>>> > >>>>>>> MIN(m) gives TableException wrapping TableException, "Cannot > >>>>>>> generate a > >>>>>>> valid execution plan for the given query: Min aggregate function > >>>>>>> does not > >>>>>>> support type: ''MAP''." The outer text names plan generation rather > >>>>>>> than > >>>>>>> the mistake, which is your second motivation. > >>>>>>> > >>>>>>> l.m < r.m gives CodeGenException, "Comparable type expected, but > was > >>>>>>> 'MAP<STRING, INT> NOT NULL'." > >>>>>>> > >>>>>>> One scope question. CodeGenException extends RuntimeException > directly > >>>>>> and > >>>>>>> carries no stability annotation, so the reparenting in the FLIP > >>>>>>> would not > >>>>>>> reach it, and it arrives as the top level exception rather than a > >>>>>>> cause. > >>>>>> Is > >>>>>>> it a later family, or worth reparenting with the others while that > is > >>>>>> open? > >>>>>>> > >>>>>>> For the message guidelines: the ''MAP'' is literal, from ten sites > in > >>>>>>> AggFunctionFactory of the form s"... type: ''$t''...". Harmless, > >>>>>>> but the > >>>>>>> sort of thing guidelines would settle. > >>>>>>> > >>>>>>> Glad to help if useful. > >>>>>>> > >>>>>>> Best regards, > >>>>>>> Sepuri Sai Krishna > >>>>>>> > >>>>>>> On Wed, Nov 4, 2026 at 3:57 PM Timo Walther via dev < > >>>>>> [email protected]> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Hi everyone, > >>>>>>>> > >>>>>>>> I would like to start a discussion about FLIP-620: Structured > >>>>>>>> errors for > >>>>>>>> Table API and SQL. > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>> https://cwiki.apache.org/confluence/spaces/FLINK/pages/462815815/ > >>>>>> FLIP-620+Structured+errors+for+Table+API+and+SQL > >>>>>>>> > >>>>>>>> Flink is increasingly embedded in larger platforms, SQL gateways, > >>>>>>>> notebooks, IDEs and AI agents. All of them need to understand why > >>>>>>>> a job > >>>>>>>> or statement failed. Today, errors are mostly free-form messages. > >>>>>>>> If you > >>>>>>>> want to react to a specific failure, you have to parse text, and > that > >>>>>>>> text can change in any release. This makes it hard to tell user > >>>>>>>> mistakes > >>>>>>>> from internal problems, to attach stable error codes, to show a > >>>>>>>> helpful > >>>>>>>> message for a given audience, or to avoid leaking sensitive values > >>>>>>>> into > >>>>>>>> logs. > >>>>>>>> > >>>>>>>> Other database systems treat errors as part of their public > contract. > >>>>>>>> They have stable identifiers, categories and SQLSTATE codes, and > the > >>>>>>>> message is only one rendering of an error. > >>>>>>>> > >>>>>>>> FLIP-620 proposes a small, backwards-compatible foundation: errors > >>>>>>>> that > >>>>>>>> have an identity, a category and structured arguments, so that > >>>>>>>> tools can > >>>>>>>> catch, classify and present them reliably. First I target Table > >>>>>>>> API and > >>>>>>>> SQL but propose a common model that other Flink components can > adopt > >>>>>> later. > >>>>>>>> > >>>>>>>> I'm looking forward to your feedback. > >>>>>>>> > >>>>>>>> Best, > >>>>>>>> Timo > >>>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>> > >>>> > >>>> > >>> > >> > > > > > >
