[v1] Add .valueOf and .values methods for enums #1628

alancai98 · 2024-10-22T22:45:56Z

Relevant Issues

[v1] Replace sprout-generated AST with hand-written AST #1610

Description

The previous sprout-generated code used Kotlin enum classes which provided some helper methods when working with enum constants -- https://kotlinlang.org/docs/enum-classes.html#working-with-enum-constants. I found I needed these in the partiql-parser migration, particularly .valueOf(<string>) and .values. This PR adds those two functions to the interface and implements them for existing enum impls.

Other Information

Updated Unreleased Section in CHANGELOG: [NO]
- No, on v1.
Any backward-incompatible changes? [YES]
- Adds to the enum interface but on v1
Any new external dependencies? [NO]
Do your changes comply with the Contributing Guidelines
and Code Style Guidelines? [YES]

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter · 2024-10-22T22:51:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (v1-migrate-ast-eq-hc@34a0993). Learn more about missing BASE report.

Additional details and impacted files

@@                   Coverage Diff                   @@
##             v1-migrate-ast-eq-hc    #1628   +/-   ##
=======================================================
  Coverage                        ?   80.03%           
  Complexity                      ?       47           
=======================================================
  Files                           ?       19           
  Lines                           ?      506           
  Branches                        ?       23           
=======================================================
  Hits                            ?      405           
  Misses                          ?       88           
  Partials                        ?       13

Flag	Coverage Δ
EXAMPLES	`80.03% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-10-22T22:53:45Z

CROSS-ENGINE-REPORT ❌

	BASE (LEGACY-V0.14.8)	TARGET (EVAL-3E005DC)	+/-
% Passing	89.67%	94.39%	4.72% ✅
Passing	5287	5565	278 ✅
Failing	609	50	-559 ✅
Ignored	0	281	281 🔶
Total Tests	5896	5896	0 ✅

Testing Details

Base Commit: v0.14.8
Base Engine: LEGACY
Target Commit: 3e005dc
Target Engine: EVAL

Result Details

❌ REGRESSION DETECTED. See Now Failing/Ignored Tests. ❌
Passing in both: 2643
Failing in both: 17
Ignored in both: 0
PASSING in BASE but now FAILING in TARGET: 3
PASSING in BASE but now IGNORED in TARGET: 108
FAILING in BASE but now PASSING in TARGET: 180
IGNORED in BASE but now PASSING in TARGET: 0

Now FAILING Tests ❌

The following 3 test(s) were previously PASSING in BASE but are now FAILING in TARGET:

Click here to see

undefinedUnqualifiedVariableWithUndefinedVariableBehaviorMissing, compileOption: PERMISSIVE
undefinedUnqualifiedVariableIsNullExprWithUndefinedVariableBehaviorMissing, compileOption: PERMISSIVE
undefinedUnqualifiedVariableIsMissingExprWithUndefinedVariableBehaviorMissing, compileOption: PERMISSIVE

Now IGNORED Tests ❌

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

Now Passing Tests

180 test(s) were previously failing in BASE (LEGACY-V0.14.8) but now pass in TARGET (EVAL-3E005DC). Before merging, confirm they are intended to pass.

The complete list can be found in GitHub CI summary, either from Step Summary or in the Artifact.

CROSS-COMMIT-REPORT ✅

	BASE (EVAL-03905DB)	TARGET (EVAL-3E005DC)	+/-
% Passing	94.39%	94.39%	0.00% ✅
Passing	5565	5565	0 ✅
Failing	50	50	0 ✅
Ignored	281	281	0 ✅
Total Tests	5896	5896	0 ✅

Testing Details

Base Commit: 03905db
Base Engine: EVAL
Target Commit: 3e005dc
Target Engine: EVAL

Result Details

Passing in both: 5565
Failing in both: 50
Ignored in both: 281
PASSING in BASE but now FAILING in TARGET: 0
PASSING in BASE but now IGNORED in TARGET: 0
FAILING in BASE but now PASSING in TARGET: 0
IGNORED in BASE but now PASSING in TARGET: 0

alancai98 · 2024-10-23T21:55:42Z

partiql-ast/src/main/java/org/partiql/ast/v1/DataType.java

@@ -1,6 +1,7 @@
 package org.partiql.ast.v1;

 import lombok.EqualsAndHashCode;
+import org.jetbrains.annotations.NotNull;

 @EqualsAndHashCode(callSuper = false)
 public class DataType implements Enum {


for every AST class implementing the enum interface, added the valueOf(String value) and values() static methods

alancai98 · 2024-10-23T21:56:29Z

partiql-ast/src/main/java/org/partiql/ast/v1/DataType.java

+            case "TIMESTAMP_WITH_TIME_ZONE": return TIMESTAMP_WITH_TIME_ZONE();
+            case "INTERVAL": return INTERVAL();
+            case "USER_DEFINED": return USER_DEFINED();
+            default: return UNKNOWN();


unidentified string values will map to UNKNOWN

this is a good example of the backwards compatibility

alancai98 · 2024-10-23T21:56:51Z

partiql-ast/src/main/java/org/partiql/ast/v1/DataType.java

+        }
+    }
+
+    public static DataType[] values() {


.values() does not include UNKNOWN

partiql-ast/src/main/java/org/partiql/ast/v1/DataType.java

partiql-ast/src/main/java/org/partiql/ast/v1/graph/GraphRestrictor.java

alancai98 · 2024-10-24T23:36:26Z

Other changes I'll make from offline discussion:

Change naming of Enum to AstEnum
Change AstEnum to be an abstract class that extends AstNode. AstNode will still be an abstract class since it has state (i.e. the tag)
Change static method values() to be named codes() and return an array of ints
Change static method valueOf(String value) to be named parse(String value). Still will return the enum class.

partiql-ast/src/main/java/org/partiql/ast/v1/AstEnum.java

RCHowell · 2024-10-25T16:39:18Z

partiql-ast/src/main/java/org/partiql/ast/v1/DataType.java

+            case "TIMESTAMP_WITH_TIME_ZONE": return TIMESTAMP_WITH_TIME_ZONE();
+            case "INTERVAL": return INTERVAL();
+            case "USER_DEFINED": return USER_DEFINED();
+            default: return UNKNOWN();


this is a good example of the backwards compatibility

partiql-ast/src/main/java/org/partiql/ast/v1/AstEnum.java

alancai98 self-assigned this Oct 22, 2024

alancai98 force-pushed the v1-migrate-ast-enum-methods branch 3 times, most recently from 215dc30 to bb30367 Compare October 22, 2024 23:20

alancai98 commented Oct 23, 2024

View reviewed changes

alancai98 marked this pull request as ready for review October 23, 2024 21:56

alancai98 requested a review from RCHowell October 23, 2024 22:15

alancai98 mentioned this pull request Oct 23, 2024

[v1] Replace sprout-generated AST with hand-written AST #1610

Open

16 tasks

RCHowell requested changes Oct 24, 2024

View reviewed changes

Base automatically changed from v1-migrate-ast-eq-hc to v1 October 24, 2024 23:45

alancai98 added 2 commits October 24, 2024 16:46

[v1] Add .valueOf and .values methods for enums

5e20948

Enum -> AstEnum; values -> codes; other refactors

5db9f62

alancai98 force-pushed the v1-migrate-ast-enum-methods branch from bb30367 to 5db9f62 Compare October 25, 2024 01:11

alancai98 requested a review from RCHowell October 25, 2024 01:15

RCHowell reviewed Oct 25, 2024

View reviewed changes

partiql-ast/src/main/java/org/partiql/ast/v1/AstEnum.java Show resolved Hide resolved

RCHowell approved these changes Oct 25, 2024

View reviewed changes

alancai98 merged commit 7f382db into v1 Oct 25, 2024
14 checks passed

alancai98 deleted the v1-migrate-ast-enum-methods branch October 25, 2024 16:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v1] Add .valueOf and .values methods for enums #1628

[v1] Add .valueOf and .values methods for enums #1628

alancai98 commented Oct 22, 2024

codecov-commenter commented Oct 22, 2024 •

edited

Loading

github-actions bot commented Oct 22, 2024 •

edited

Loading

alancai98 Oct 23, 2024

alancai98 Oct 23, 2024

RCHowell Oct 25, 2024

alancai98 Oct 23, 2024

alancai98 commented Oct 24, 2024

RCHowell Oct 25, 2024

[v1] Add .valueOf and .values methods for enums #1628

[v1] Add .valueOf and .values methods for enums #1628

Conversation

alancai98 commented Oct 22, 2024

Relevant Issues

Description

Other Information

License Information

codecov-commenter commented Oct 22, 2024 • edited Loading

Codecov Report

github-actions bot commented Oct 22, 2024 • edited Loading

CROSS-ENGINE-REPORT ❌

Testing Details

Result Details

Now FAILING Tests ❌

Now IGNORED Tests ❌

Now Passing Tests

CROSS-COMMIT-REPORT ✅

Testing Details

Result Details

alancai98 Oct 23, 2024

Choose a reason for hiding this comment

alancai98 Oct 23, 2024

Choose a reason for hiding this comment

RCHowell Oct 25, 2024

Choose a reason for hiding this comment

alancai98 Oct 23, 2024

Choose a reason for hiding this comment

alancai98 commented Oct 24, 2024

RCHowell Oct 25, 2024

Choose a reason for hiding this comment

codecov-commenter commented Oct 22, 2024 •

edited

Loading

github-actions bot commented Oct 22, 2024 •

edited

Loading