Skip to content

Conversation

@kmozaid
Copy link
Owner

@kmozaid kmozaid commented May 6, 2022

Instructions:

  1. The PR has to be tagged with at least one of the following labels (*):
    1. feature
    2. bugfix
    3. performance
    4. ui
    5. backward-incompat
    6. release-notes (**)
  2. Remove these instructions before publishing the PR.

(*) Other labels to consider:

  • testing
  • dependencies
  • docker
  • kubernetes
  • observability
  • security
  • code-style
  • extension-point
  • refactor
  • cleanup

(**) Use release-notes label for scenarios like:

  • New configuration options
  • Deprecation of configurations
  • Signature changes to public methods/interfaces
  • New plugins added or old plugins removed

walterddr and others added 5 commits April 6, 2022 11:42
* add pinot-query-planner

- fix calcite upgrade compilation issue
- fix query compilation runtime after calcite 1.29 upgrade
- linter

* address diff comments and add more TODOs

Co-authored-by: Rong Rong <[email protected]>
* add pinot-query-runtime

- inter-stage exchange interface
- worker executor interface
- dispatcher and dispatch-receiving server interface
- row-based runtime operator (incomplete)

several component that's need in order to access current pinot-server
functionality
- converter between stage plan into existing PinotQuery/QueryContext
- after-execution exchange sender connected to InstanceResponseOperator

* fix jdk8 compile

* documentation for mailbox module

* document the runtime

* change stageId to Integer

* change jobId to also long type to align with current pinot requestId

* change the serializable format of dispatchable QueryPlan

Work Items Done
1. split worker.proto into the service proto and the message proto
2. created StageRoot proto object that can encode the input StageNodes as serialized bytes.
3. modify ser/deser modules in query-runtime
4.remove unnecessary plan node operations in stage plan

Work Items TODO
1. make a proto for StageNode, should include all possible StageNode Fields.
2. make a serializer and deserializer (real one to replace the current
   java.io.Serializable)

* remove plan.proto

will add it back once the serializable StageNode format is determined

* fix serde

* address diff comments

Co-authored-by: Rong Rong <[email protected]>
* serializable format

* fix linter

* bump version since feature branch based bumped

* fix auto-rebase error

* fix test async mock intercept issue

* use reflection for ser/de

* also fix test coverage on all node types

* fix java8

* fix JOINNode member type as well as plan.proto comments

Co-authored-by: Rong Rong <[email protected]>
* make planner node ready for project/filter pushdown

making filter expression compilation work, generating operator

add ProjectNode compilation as well

* fixing comments from previous PR

- add float type
- change serde javadoc
- change function name for StageNode serializable
- change function name in proto utils

* add distributed hash join capability

* fixing serde

* refactor calcite components into sql/rex/expression 3 subclasses

* address diff comments

1. renamed BroadcastJoin to HashJoin
2. relocate packages inside planner to planner.node to planner.logical/serde/stage based on their functionalities
3. added SqlHint strategy to planner so that it can do either hash or broadcast join

* additional comment address

1. remove CalciteExpressionParser as it is not used
2. remove rowType in construction for AbstractStagerNode

* rename RelHint

Co-authored-by: Rong Rong <[email protected]>
@kmozaid kmozaid closed this Dec 21, 2022
kmozaid pushed a commit that referenced this pull request Aug 11, 2025
* Addition of initial spi change checker code

* Fixes to yaml file, mainly excluding artifacts.zip code to test run further

* changing shell file permissions

* add "exit 1" to shell script and mess with TableConfig method signature

* changing shell file permissions again for some reason

* changing git diff checker file path

* second git diff checker file path change

* removing unnecessary code from files, fixed issue with running GitDiffChecker, added functionality for displaying line number, and reverted temporary change to TableConfig.java

* permission changes

* changing permissions

* fixing compilation error

* fixing parameterization of commits

* trial and error yml file #1

* commit for testing that config file correctly has parameters, changing sh file so that "No incorrect..." message only displayed once, and adding blank line check to GitDiffChecker

* GitDiffChecker: add case to skip ---. otherwise, change parameters to work with pull requests.

* testing

* testing #2

* Revert "testing #2"

This reverts commit 270937f.

* yml: change main to master. shell: change main to master and change conditional to reflect return type change. java: switch from returning line number to string of code, as previous logic did not work if multiple "chunks" of code were changed, and added annotation logic that excluded json-related annotations.

* yml: comment out apache/pinot condition for now. shell: change error message and put text file in my module. java: slightly altered regex after rethinking it, removed System.out.println, i think it's not necessary

* java: slightly altered regex to account for interface definitions using semicolons and not curly braces

* changing "no incorrect spi changes" value to "0", so if the method returns nothing, there isn't an accidental test passing

* minor change to annotation regex, \n changed to $ (end of string)

* fix to pom.xml

* Added logic for outputting line number along with original file code snippet

* per testing on another branch, slightly updating line number logic

* Removed my outdated custom Java implementation of pinot-spi change checking. Switched to the japicmp plugin, with some modifications to the compatibility of checks that japicmp performs. With this, contributors will be able to see if they made incompatible SPI changes when running mvn clean, rather than waiting until they make a PR. Adding a .jar of pinot-spi that japicmp will use for comparisons.

* Per Tianle's comment, added a comment in the pom file explaining our justification for using a baseline jar for comparison, and that we need to eventually transition away from it.

* Removed code that made annotation changes/deletions incompatible. Fixed pom file so that all pinot-spi files are checked

* updated baseline .jar to match updates from apache:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants