-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-15238: [C++] ARROW_ENGINE module with substrait consumer #12279
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
Closed
Closed
Changes from all commits
Commits
Show all changes
116 commits
Select commit
Hold shift + click to select a range
ff51e35
first pass at using substrait protobufs
bkietz 906aacf
add conversion of types and a basic roundtrip test
bkietz 5fc6c34
reorganize to engine/substrait/{proto}_internal.cc,h
bkietz 151cf72
add basic literal conversions
bkietz 1cf679b
don't rely on Datum::type in DataEq matcher
bkietz 6dce17b
add substrait_gen_verify, allow configurable substrait repo and tag
bkietz 53c7c7d
Expose NamedStruct<=>Schema serde
bkietz cd919f3
add if_else <-> IfThen conversion
bkietz c079359
rebase, catch up to changes in substrait
bkietz 5c69fb1
finish catching up with substrait's new field references
bkietz 2a77d28
add SubstraitFromJSON
bkietz a5c25de
port more tests to JSON
bkietz 873ec23
add more DataEq matchers
bkietz 05be909
use Date64 for substrait::date
bkietz 7309964
add extension types for interval_*, support deeply nested struct fiel…
bkietz 27af6b6
add basic sketch of ExtensionSet for tracking substrait extensions
bkietz 37a62de
get a failing test for arrow::null
bkietz 8397ee5
refactor extension types to index variations alongside types
bkietz 61feb19
use an ExtensionSet-local registry
bkietz 1eb4bc0
add ExtensionSet <-> Plan factories
bkietz e63da1e
pre merge stash
bkietz f37084a
post rebase cleanup
bkietz 2725ed7
Changes needed to get branch to compile
jvanstraten 11e7f3f
Fix uninitialized pointers
jvanstraten 96727e1
Fix FieldRef/StructField order
jvanstraten 07b259c
gcc: use a pointer to the properties tuple instead of a reference
bkietz 885836a
advance substrait version
bkietz 0b94f3a
make JSON utils public, add CheckMessagesEquivalent()
bkietz 98c74a8
Revert now unnecessary part of 9255fb6
jvanstraten 6fd2e73
Support nested StructFields
jvanstraten 0034651
Support struct_field compute function
jvanstraten b5e6fa4
Use ReferenceSegment.child where possible when emitting Substrait
jvanstraten e7184f5
Use CheckMessagesEquivalent() for test
jvanstraten bf93511
Fix compilation with googletest 1.11
jvanstraten 2573bc5
add nullable field roundtrip test
bkietz 095560f
Use lowercase nullptr in cc files
jvanstraten 24517ff
Remove redundant else block
jvanstraten c4d9877
Fix clang-format'ing
jvanstraten f2e0e71
Simplify Fingerprintable constructor
jvanstraten b09d372
Simplify unique_ptr moves and casts
jvanstraten 1557f4f
Minor fixes in suggested changes
jvanstraten 839826b
Add tests for mixed struct references and expressions
jvanstraten d12545f
clean up internal::
bkietz 9dd9c70
revert Fingerprintable change
bkietz 9a569c9
add a simple example of substrait consumption
bkietz ee32bb5
add sketch of Relation conversion
bkietz cf33bd1
WIP on case_when support
jvanstraten 2fc0123
Fully implement case_when(make_struct(...), ...)
jvanstraten 9468920
Simplify ReferenceSegment manipulation functions
jvanstraten 4da0939
add test for ReadRel conversion
bkietz 1487b37
add function extensions to ExtensionSet
bkietz d237377
Add a test for extraction of an ExtensionSet from a Plan
bkietz 5e0d6e5
add a roundtrip test for calling an extension function
bkietz cd22ef0
repair status_test::MatcherExplanations
bkietz 1dc4c9d
removing old generated extension files
bkietz ed5b0d5
unity: ensure globals are unique within a TU
bkietz 5d61724
ensure generated files are also excluded from lint_cpp_cli
bkietz 37b5673
substrait consumer api cleanup
bkietz 5b362f9
ensure generated files are also excluded from rat
bkietz b1a9bba
put actual json in engine_substrait_consumption.cc
bkietz b6499ae
msvc: suppress C4251 (needs dll-interface)
bkietz 1473dc2
remove duplicate ARROW_ENGINE option
bkietz 107c5b7
ensure ARROW_DATASET is available if ARROW_ENGINE=on
bkietz 5012be6
try to localize suppressions, try defining LIBPROTOBUF_EXPORTS
bkietz 2c73334
run cmake-format
bkietz 65c2ee8
msvc: just suppress C4251, more int/size_t fixes
bkietz 6d8ebc7
msvc: one more int/size_t fix
bkietz adfe196
msvc: one more int/size_t fix
bkietz 0a965c5
use libprotobuf 3.19
bkietz ecb5214
Remove files generated by protoc
jvanstraten aa0e94b
Split ARROW_SUBSTRAIT_REPO_AND_TAG option
jvanstraten 584b49e
Reorganize Arrow/Substrait example
jvanstraten ba74f7d
Remove redundant :: namespace prefixes in Arrow/Substrait example
jvanstraten f1e6259
Clarify comment in Arrow/Substrait example
jvanstraten 283e87b
Document IgnoringConsumer::tag_ in Arrow/Substrait example
jvanstraten d64d53b
Clarify comment in Arrow/Substrait example
jvanstraten e730309
Clarify Arrow/Substrait example; remove unnecessary implementation de…
jvanstraten 2901721
Clarify comment in Arrow/Substrait example
jvanstraten 96aebea
Simplify includes for Arrow/Substrait example
jvanstraten a8dd172
Revert error message
jvanstraten 1404f71
Remove duplicate definition of u32
jvanstraten 4b92961
Minor improvements to expression_internal.cc based on code review
jvanstraten a9862a1
Fix std::string(n,c) argument order in Substrait example
jvanstraten 4b55dee
Avoid shared_ptr copy, fix move -> forward in datum.h
jvanstraten 7f81492
Add convenience constructors from std::string to *BinaryScalars
jvanstraten a284e18
Map Substrait date to Arrow Date32 instead of Date64
jvanstraten d099775
Clarify lack of struct field names in Substrait in comment as requested
jvanstraten bdb0b74
Slightly improve Substrait to Arrow conversion error messages
jvanstraten 9453707
Add docstrings to functions defined in serde.h
jvanstraten c58b9c6
Add missing NotImplemented checks for Substrait FileOrFiles message
jvanstraten fc3d480
Remove unused associated type
jvanstraten 2da3023
Fix parameter name links in docstrings added in 9453707
jvanstraten 231f95a
ARROW_ENGINE currently depends on ARROW_PARQUET (only place it can ge…
jvanstraten e7ec32d
Fix CMake module style violations
jvanstraten cbc9842
Fix correspondence between extension_types.yaml and extension_set.cc,…
jvanstraten 736c7a6
Fix make_shared<T> where T = universal reference
jvanstraten 15fdc58
Clarify comment
jvanstraten 22ad42f
Remove unnecessary imports in extension_types.cc
jvanstraten bae1822
Document plan_internal.h
jvanstraten 6240ca2
Disable ARROW_ENGINE in CI job where ARROW_PARQUET doesn't work (whic…
jvanstraten b8ff996
Improve docstring
jvanstraten f4b351f
Remove usage of scan_options->projection
jvanstraten 709ab65
Remove unused test helper function
jvanstraten c8e93ea
Add test for round-tripping Substrait Map<K,V>, and fix implementation
jvanstraten fc9d1a9
Added doc strings for SimpleExtensionType
westonpace 8bd00a2
Made a simplification pass on the extension set. I added some commen…
westonpace 2156716
Remove questionable SubstraitToJson & SubstraitFromJson test helper f…
jvanstraten f5483f4
Removing unneeded protobuf cmake flag
westonpace 1b8ef91
Fix up the substrait consumption example to take in a filename instea…
westonpace 78d0fca
Update cpp/src/arrow/engine/substrait/extension_set.h
westonpace 2b0131c
Cleaned up comments per PR review
westonpace cde0b6f
Merge pull request #1 from westonpace/substrait-consumer
jvanstraten a4a5f7a
Merge pull request #2 from westonpace/substrait-consumer-build-fix
jvanstraten b7a2a1e
Fix lint violation
jvanstraten 90d7ae3
If we build protobuf from source we always build it statically. We n…
westonpace fe1f784
Merge pull request #3 from westonpace/substrait-consumer-build-fix-2
jvanstraten File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| # - Find Arrow Engine (arrow/engine/api.h, libarrow_engine.a, libarrow_engine.so) | ||
| # | ||
| # This module requires Arrow from which it uses | ||
| # arrow_find_package() | ||
| # | ||
| # This module defines | ||
| # ARROW_ENGINE_FOUND, whether Arrow Engine has been found | ||
| # ARROW_ENGINE_IMPORT_LIB, | ||
| # path to libarrow_engine's import library (Windows only) | ||
| # ARROW_ENGINE_INCLUDE_DIR, directory containing headers | ||
| # ARROW_ENGINE_LIB_DIR, directory containing Arrow Engine libraries | ||
| # ARROW_ENGINE_SHARED_LIB, path to libarrow_engine's shared library | ||
| # ARROW_ENGINE_STATIC_LIB, path to libarrow_engine.a | ||
|
|
||
| if(DEFINED ARROW_ENGINE_FOUND) | ||
| return() | ||
| endif() | ||
|
|
||
| set(find_package_arguments) | ||
| if(${CMAKE_FIND_PACKAGE_NAME}_FIND_VERSION) | ||
| list(APPEND find_package_arguments "${${CMAKE_FIND_PACKAGE_NAME}_FIND_VERSION}") | ||
| endif() | ||
| if(${CMAKE_FIND_PACKAGE_NAME}_FIND_REQUIRED) | ||
| list(APPEND find_package_arguments REQUIRED) | ||
| endif() | ||
| if(${CMAKE_FIND_PACKAGE_NAME}_FIND_QUIETLY) | ||
| list(APPEND find_package_arguments QUIET) | ||
| endif() | ||
| find_package(Arrow ${find_package_arguments}) | ||
| find_package(Parquet ${find_package_arguments}) | ||
|
|
||
| if(ARROW_FOUND AND PARQUET_FOUND) | ||
| arrow_find_package(ARROW_ENGINE | ||
| "${ARROW_HOME}" | ||
| arrow_engine | ||
| arrow/engine/api.h | ||
| ArrowEngine | ||
| arrow-engine) | ||
| if(NOT ARROW_ENGINE_VERSION) | ||
| set(ARROW_ENGINE_VERSION "${ARROW_VERSION}") | ||
| endif() | ||
| endif() | ||
|
|
||
| if("${ARROW_ENGINE_VERSION}" VERSION_EQUAL "${ARROW_VERSION}") | ||
| set(ARROW_ENGINE_VERSION_MATCH TRUE) | ||
| else() | ||
| set(ARROW_ENGINE_VERSION_MATCH FALSE) | ||
| endif() | ||
|
|
||
| mark_as_advanced(ARROW_ENGINE_IMPORT_LIB | ||
| ARROW_ENGINE_INCLUDE_DIR | ||
| ARROW_ENGINE_LIBS | ||
| ARROW_ENGINE_LIB_DIR | ||
| ARROW_ENGINE_SHARED_IMP_LIB | ||
| ARROW_ENGINE_SHARED_LIB | ||
| ARROW_ENGINE_STATIC_LIB | ||
| ARROW_ENGINE_VERSION | ||
| ARROW_ENGINE_VERSION_MATCH) | ||
|
|
||
| find_package_handle_standard_args( | ||
| ArrowEngine | ||
| REQUIRED_VARS ARROW_ENGINE_INCLUDE_DIR ARROW_ENGINE_LIB_DIR ARROW_ENGINE_VERSION_MATCH | ||
| VERSION_VAR ARROW_ENGINE_VERSION) | ||
| set(ARROW_ENGINE_FOUND ${ArrowEngine_FOUND}) | ||
|
|
||
| if(ArrowEngine_FOUND AND NOT ArrowEngine_FIND_QUIETLY) | ||
| message(STATUS "Found the Arrow Engine by ${ARROW_ENGINE_FIND_APPROACH}") | ||
| message(STATUS "Found the Arrow Engine shared library: ${ARROW_ENGINE_SHARED_LIB}") | ||
| message(STATUS "Found the Arrow Engine import library: ${ARROW_ENGINE_IMPORT_LIB}") | ||
| message(STATUS "Found the Arrow Engine static library: ${ARROW_ENGINE_STATIC_LIB}") | ||
| endif() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| // Licensed to the Apache Software Foundation (ASF) under one | ||
| // or more contributor license agreements. See the NOTICE file | ||
| // distributed with this work for additional information | ||
| // regarding copyright ownership. The ASF licenses this file | ||
| // to you under the Apache License, Version 2.0 (the | ||
| // "License"); you may not use this file except in compliance | ||
| // with the License. You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, | ||
| // software distributed under the License is distributed on an | ||
| // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| // KIND, either express or implied. See the License for the | ||
| // specific language governing permissions and limitations | ||
| // under the License. | ||
|
|
||
| #include <arrow/api.h> | ||
| #include <arrow/compute/api.h> | ||
| #include <arrow/engine/api.h> | ||
|
|
||
| #include <cstdlib> | ||
| #include <iostream> | ||
| #include <memory> | ||
| #include <vector> | ||
|
|
||
| namespace eng = arrow::engine; | ||
| namespace cp = arrow::compute; | ||
|
|
||
| #define ABORT_ON_FAILURE(expr) \ | ||
westonpace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| do { \ | ||
| arrow::Status status_ = (expr); \ | ||
| if (!status_.ok()) { \ | ||
| std::cerr << status_.message() << std::endl; \ | ||
| abort(); \ | ||
| } \ | ||
| } while (0); | ||
|
|
||
| class IgnoringConsumer : public cp::SinkNodeConsumer { | ||
westonpace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| public: | ||
| explicit IgnoringConsumer(size_t tag) : tag_{tag} {} | ||
|
|
||
| arrow::Status Consume(cp::ExecBatch batch) override { | ||
| // Consume a batch of data | ||
| // (just print its row count to stdout) | ||
| std::cout << "-" << tag_ << " consumed " << batch.length << " rows" << std::endl; | ||
| return arrow::Status::OK(); | ||
| } | ||
|
|
||
| arrow::Future<> Finish() override { | ||
| // Signal to the consumer that the last batch has been delivered | ||
| // (we don't do any real work in this consumer so mark it finished immediately) | ||
| // | ||
| // The returned future should only finish when all outstanding tasks have completed | ||
| // (after this method is called Consume is guaranteed not to be called again) | ||
| std::cout << "-" << tag_ << " finished" << std::endl; | ||
| return arrow::Future<>::MakeFinished(); | ||
| } | ||
|
|
||
| private: | ||
| // A unique label for instances to help distinguish logging output if a plan has | ||
| // multiple sinks | ||
| // | ||
| // In this example, this is set to the zero-based index of the relation tree in the plan | ||
| size_t tag_; | ||
westonpace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| arrow::Future<std::shared_ptr<arrow::Buffer>> GetSubstraitFromServer( | ||
| const std::string& filename) { | ||
| // Emulate server interaction by parsing hard coded JSON | ||
| std::string substrait_json = R"({ | ||
| "relations": [ | ||
| {"rel": { | ||
| "read": { | ||
| "base_schema": { | ||
| "struct": { | ||
| "types": [ {"i64": {}}, {"bool": {}} ] | ||
| }, | ||
| "names": ["i", "b"] | ||
| }, | ||
| "local_files": { | ||
| "items": [ | ||
| { | ||
| "uri_file": "file://FILENAME_PLACEHOLDER", | ||
| "format": "FILE_FORMAT_PARQUET" | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| }} | ||
| ], | ||
| "extension_uris": [ | ||
| { | ||
| "extension_uri_anchor": 7, | ||
| "uri": "https://github.com/apache/arrow/blob/master/format/substrait/extension_types.yaml" | ||
| } | ||
| ], | ||
| "extensions": [ | ||
| {"extension_type": { | ||
| "extension_uri_reference": 7, | ||
| "type_anchor": 42, | ||
| "name": "null" | ||
| }}, | ||
| {"extension_type_variation": { | ||
| "extension_uri_reference": 7, | ||
| "type_variation_anchor": 23, | ||
| "name": "u8" | ||
| }}, | ||
| {"extension_function": { | ||
| "extension_uri_reference": 7, | ||
| "function_anchor": 42, | ||
| "name": "add" | ||
| }} | ||
| ] | ||
westonpace marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| })"; | ||
| std::string filename_placeholder = "FILENAME_PLACEHOLDER"; | ||
| substrait_json.replace(substrait_json.find(filename_placeholder), | ||
| filename_placeholder.size(), filename); | ||
| return eng::internal::SubstraitFromJSON("Plan", substrait_json); | ||
| } | ||
|
|
||
| int main(int argc, char** argv) { | ||
| if (argc < 2) { | ||
| std::cout << "Please specify a parquet file to scan" << std::endl; | ||
| // Fake pass for CI | ||
| return EXIT_SUCCESS; | ||
| } | ||
|
|
||
| // Plans arrive at the consumer serialized in a Buffer, using the binary protobuf | ||
| // serialization of a substrait Plan | ||
| auto maybe_serialized_plan = GetSubstraitFromServer(argv[1]).result(); | ||
| ABORT_ON_FAILURE(maybe_serialized_plan.status()); | ||
| std::shared_ptr<arrow::Buffer> serialized_plan = | ||
| std::move(maybe_serialized_plan).ValueOrDie(); | ||
|
|
||
| // Print the received plan to stdout as JSON | ||
| arrow::Result<std::string> maybe_plan_json = | ||
| eng::internal::SubstraitToJSON("Plan", *serialized_plan); | ||
| ABORT_ON_FAILURE(maybe_plan_json.status()); | ||
| std::cout << std::string(50, '#') << " received substrait::Plan:" << std::endl; | ||
| std::cout << maybe_plan_json.ValueOrDie() << std::endl; | ||
|
|
||
| // The data sink(s) for plans is/are implicit in substrait plans, but explicit in | ||
| // Arrow. Therefore, deserializing a plan requires a factory for consumers: each | ||
| // time the root of a substrait relation tree is deserialized, an Arrow consumer is | ||
| // constructed into which its batches will be piped. | ||
| std::vector<std::shared_ptr<cp::SinkNodeConsumer>> consumers; | ||
| std::function<std::shared_ptr<cp::SinkNodeConsumer>()> consumer_factory = [&] { | ||
| // All batches produced by the plan will be fed into IgnoringConsumers: | ||
| auto tag = consumers.size(); | ||
| consumers.emplace_back(new IgnoringConsumer{tag}); | ||
| return consumers.back(); | ||
| }; | ||
|
|
||
| // Deserialize each relation tree in the substrait plan to an Arrow compute Declaration | ||
| arrow::Result<std::vector<cp::Declaration>> maybe_decls = | ||
| eng::DeserializePlan(*serialized_plan, consumer_factory); | ||
| ABORT_ON_FAILURE(maybe_decls.status()); | ||
| std::vector<cp::Declaration> decls = std::move(maybe_decls).ValueOrDie(); | ||
|
|
||
| // It's safe to drop the serialized plan; we don't leave references to its memory | ||
| serialized_plan.reset(); | ||
|
|
||
| // Construct an empty plan (note: configure Function registry and ThreadPool here) | ||
| arrow::Result<std::shared_ptr<cp::ExecPlan>> maybe_plan = cp::ExecPlan::Make(); | ||
| ABORT_ON_FAILURE(maybe_plan.status()); | ||
| std::shared_ptr<cp::ExecPlan> plan = std::move(maybe_plan).ValueOrDie(); | ||
|
|
||
| // Add decls to plan (note: configure ExecNode registry before this point) | ||
| for (const cp::Declaration& decl : decls) { | ||
| ABORT_ON_FAILURE(decl.AddToPlan(plan.get()).status()); | ||
| } | ||
|
|
||
| // Validate the plan and print it to stdout | ||
| ABORT_ON_FAILURE(plan->Validate()); | ||
| std::cout << std::string(50, '#') << " produced arrow::ExecPlan:" << std::endl; | ||
| std::cout << plan->ToString() << std::endl; | ||
|
|
||
| // Start the plan... | ||
| std::cout << std::string(50, '#') << " consuming batches:" << std::endl; | ||
| ABORT_ON_FAILURE(plan->StartProducing()); | ||
|
|
||
| // ... and wait for it to finish | ||
| ABORT_ON_FAILURE(plan->finished().status()); | ||
| return EXIT_SUCCESS; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.