Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use pattern matching for instanceof in iceberg connector #15488

Merged

Conversation

krvikash
Copy link
Contributor

@krvikash krvikash commented Dec 21, 2022

Description

Use pattern matching for instanceof in iceberg connector

Pattern matching for instanceof introduced in Java 16 (https://openjdk.org/jeps/394)

Motivation (copied from https://openjdk.org/jeps/394)
Nearly every program includes some sort of logic that combines testing if an expression has a certain type or structure, and then conditionally extracting components of its state for further processing. For example, all Java programmers are familiar with the instanceof-and-cast idiom:

if (obj instanceof String) {
    String s = (String) obj;    // grr...
    ...
}

There are three things going on here: a test (is obj a String?), a conversion (casting obj to String), and the declaration of a new local variable (s) so that we can use the string value. This pattern is straightforward and understood by all Java programmers, but is suboptimal for several reasons. It is tedious; doing both the type test and cast should be unnecessary (what else would you do after an instanceof test?). This boilerplate — in particular, the three occurrences of the type String — obfuscates the more significant logic that follows. But most importantly, the repetition provides opportunities for errors to creep unnoticed into programs.

Rather than reach for ad-hoc solutions, we believe it is time for Java to embrace pattern matching. Pattern matching allows the desired "shape" of an object to be expressed concisely (the pattern), and for various statements and expressions to test that "shape" against their input (the matching). Many languages, from Haskell to C#, have embraced pattern matching for its brevity and safety.

Additional context and related issues

NA

Release notes

(X) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Dec 21, 2022
@krvikash krvikash self-assigned this Dec 21, 2022
@findepi findepi requested a review from ebyhr December 21, 2022 13:16
@krvikash krvikash added the no-release-notes This pull request does not require release notes entry label Dec 21, 2022
@findinpath
Copy link
Contributor

Do address also:

@findinpath findinpath self-requested a review December 21, 2022 15:30
@krvikash krvikash force-pushed the use-pattern-matching-variable-for-instanceOf branch from 99a5ab6 to a115789 Compare December 21, 2022 15:43
@krvikash krvikash changed the title Use pattern matching for instanceOf in iceberg connector Use pattern matching for instanceof in iceberg connector Dec 21, 2022
@krvikash krvikash force-pushed the use-pattern-matching-variable-for-instanceOf branch 2 times, most recently from 1a50cbd to db023e7 Compare December 21, 2022 20:21
@krvikash krvikash force-pushed the use-pattern-matching-variable-for-instanceOf branch 3 times, most recently from 4a67cb1 to 1835cde Compare December 23, 2022 08:51
@krvikash krvikash force-pushed the use-pattern-matching-variable-for-instanceOf branch from 1835cde to 572015a Compare December 23, 2022 08:52
@ebyhr
Copy link
Member

ebyhr commented Dec 23, 2022

CI hit #15293

@ebyhr ebyhr merged commit 6092ef4 into trinodb:master Dec 23, 2022
@github-actions github-actions bot added this to the 404 milestone Dec 23, 2022
@krvikash krvikash deleted the use-pattern-matching-variable-for-instanceOf branch December 23, 2022 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed no-release-notes This pull request does not require release notes entry
Development

Successfully merging this pull request may close these issues.

3 participants