Update from Velox main (2025-04-17) #3
Merged
Conversation
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
Summary: Integrate new shared data and overall refactor. Pull Request resolved: facebookincubator#12668 Reviewed By: kKPulla Differential Revision: D71413018 Pulled By: kagamiori fbshipit-source-id: 9627bb500876a607d4cde6b4d9928f1a4d2236c6
…ncDataCacheTest.cpp
Summary:
`-Wunused-exception-parameter` has identified an unused exception parameter. This diff removes it.
This:
```
try {
...
} catch (exception& e) {
// no use of e
}
```
should instead be written as
```
} catch (exception&) {
```
If the code compiles, this is safe to land.
Reviewed By: meyering
Differential Revision: D71503173
fbshipit-source-id: 37beb04da69370b72cde0a227a63d5863de497d4
Summary: We need Abseil 20240116 or later for RE2 2024-07-02. Because RE2 uses `ABSL_LOG(DFATAL)` and it needs this commit: abseil/abseil-cpp@70172ad Fixes facebookincubator#12674 Pull Request resolved: facebookincubator#12678 Reviewed By: bikramSingh91 Differential Revision: D71531109 Pulled By: xiaoxmeng fbshipit-source-id: e9a9a93df802c5752d1f8aaf21ef6eafa0b1365a
…cebookincubator#12576) Summary: https://github.com/apache/spark/blob/84f5fd99fef1ad3a2b2c41dbee09c60560479c11/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala#L2341 Pull Request resolved: facebookincubator#12576 Reviewed By: bikramSingh91 Differential Revision: D71479672 Pulled By: xiaoxmeng fbshipit-source-id: 8e5471ac471886aeb7fa14446caf2c9d2dd0d517
…acebookincubator#12712) Summary: Pull Request resolved: facebookincubator#12712 Enable overrides for getPreferredSize behavior on a per memory pool basis Reviewed By: xiaoxmeng Differential Revision: D71427371 fbshipit-source-id: ce4e1c9682de7d00af97ca7eb9a6323c4c8ffd02
Summary: Pull Request resolved: facebookincubator#12588 Implement `encodedVectorCopy`, a generic vector copy utility that preserves encodings for memory saving purpose. ## Encoding Preservation There are mainly 2 use cases for this new function. One is to merge multiple encoded vectors (`source`s) into one large encoded vector (`target`); the other is to update specific rows (`source`) in a large vector (`target`), while keeping the encodings. Both use cases requires us to keep the encoding on `target`, so it is decided as the behavior of this function. There are some exceptions to this rule: - We merge multiple adjacent layers of dictionary and constant wrappers into one. - When the values type size in dictionary is no larger than the index type, we flatten the vector to save memory. - When `target` is constant, we convert it to dictionary to allow different values in `source`. - When `target` is flat ROW, MAP, or ARRAY, and `source` is constant or dictionary encoded, the result will be dictionary encoded, to avoid flattening the child vectors. Once the target becomes dictionary, it can stay that way and we can keep adding new content to it while keeping the encoding, this is a typical use case for encoding preserved merging. ## Inner Vector Compaction Other than encoding, we also pay special attention to avoid holding on memory that is no longer needed. This is especially important for the merging use case, as the `target` gets updated, majority rows of its inner vectors will be dereferenced and no longer used. There are 2 cases where we need to take care of this. The first is for dictionary encoding, some rows in the alphabet (base/value) vector become no longer referenced by the indices, so we should recycle them. This is done properly that when we translate the copy ranges on dictionary indices to the copy ranges on alphabet, we overwrite the unused rows in alphabet using the new alphabet rows from source. This way we efficiently reuse the memory in alphabet without both reallocation and memory leaking. The second case is for `ARRAY`/`MAP`, the elements/keys/values vector can have rows that are no longer referenced from the parent. This is a little harder to solve than in the dictionary case, since the nested rows need to be contiguous for one parent row (offset/size pair), which means we cannot move them around easily. Our approach is to allow some unused nested rows, but keep track of the percentage of them, and once they exceed certain threshold (50% by default and configurable), we make a new copy of the nested vector and copy only the used rows over. This allows us to reuse the nested rows to a certain degree while keep some bounds on the memory usage. Reviewed By: mbasmanova Differential Revision: D70867237 fbshipit-source-id: 0cddd37fd7188d89ea541fb89324aa9a10745415
) Summary: Pull Request resolved: facebookincubator#12708 These functions allow users to find the parent or children tiles of a BingTile. Reviewed By: bikramSingh91 Differential Revision: D71413543 fbshipit-source-id: bcfa7f5a175362d22ff3542d1b3e0b922eb1ff18
…task failure in pyspark (facebookincubator#12729) Summary: Pull Request resolved: facebookincubator#12729 Here is the race condition can cause memory leak in race between async direct coalesce load and early task failure: T1. file reader triggers stripe load which does coalesce prefetch T2. coalesce prefetch kicks off and pass the cancellation check T3. coalesce load does the memory allocations and do read from storage T4. before table scan do on-demand load, the task fails or prefetch data is skipped by filtering and task finishes. T5. task destruction frees the memory pool hits the memory leak check failure. T6. if we disable the memory leak check failure (with memory leak metric reporting in Meta production), then the buffer free will throw with bad memory pool pointer. Verified the fix with unit test that reproduce the race condition. This is exposed by pyspark use case. Reviewed By: tanjialiang, oerling Differential Revision: D71529929 fbshipit-source-id: de1c545c48dc23602a79238b878020ce2f29c4f4
…2742) Summary: VeloxCon 2025 registration banner Pull Request resolved: facebookincubator#12742 Reviewed By: xiaoxmeng Differential Revision: D71580661 Pulled By: pedroerp fbshipit-source-id: 7815d4ad835cf4b79c7ed81ce0383b9b0050bb36
…ubator#12741) Summary: Pull Request resolved: facebookincubator#12741 Currently when given a query like below, velox doesnt round , but rounds down which gives us different results compared to java. ``` SELECT "json_extract"(metrics, '$.eventTimings'), CAST("json_extract"(metrics, '$.eventTimings') AS ARRAY(ROW(inputdelay BIGINT))), ANY_MATCH( CAST( "json_extract"(metrics, '$.eventTimings') AS ARRAY(ROW(inputdelay BIGINT)) ), (x) -> (x.inputDelay >= 300) ) FROM ( VALUES '{ "eventTimings": [ { "inputDelay": 299.5999999996274} ] }' ) AS t(metrics) ``` Java returns 300 and velox 299. This PR fixes this. Reviewed By: kevinwilfong Differential Revision: D71570565 fbshipit-source-id: e7493578760e8797c7268cd9864954e0db3e3bb7
… suppress llvm-19-exposed -Wmissing-template-arg-list-after-template-kw warning (facebookincubator#12744) Summary: Pull Request resolved: facebookincubator#12744 This avoids the following errors: velox/functions/prestosql/aggregates/BitwiseAggregates.cpp:91:47: error: a template argument list is expected after a name prefixed by the template keyword [-Wmissing-template-arg-list-after-template-kw] Reviewed By: pedroerp, dtolnay Differential Revision: D71579788 fbshipit-source-id: 4d9107f4086aed060ceae64bd1348cc37da402dd
…or#12624) Summary: Pull Request resolved: facebookincubator#12624 When a small participant requesting for a large amount of capacity, we shall take into consideration of the requesting capacity make it more prone for being killed by global arbitration. This would make global arbitration victim selection more fair. Reviewed By: xiaoxmeng Differential Revision: D71076051 fbshipit-source-id: 153cf40ecc06829dee20b4006305943f3fbb84af
… -Wunused-but-set-variable warnings (facebookincubator#12743) Summary: Pull Request resolved: facebookincubator#12743 This avoids the following errors: velox/exec/tests/utils/PlanBuilder.cpp:2117:14: error: variable 'constant' set but not used [-Werror,-Wunused-but-set-variable] velox/exec/tests/utils/PlanBuilder.cpp:2113:14: error: variable 'field' set but not used [-Werror,-Wunused-but-set-variable] Reviewed By: pedroerp, dtolnay Differential Revision: D71579752 fbshipit-source-id: 8ab46b889c8193984dd4d30cc9eb92c0982381ed
Summary: Pull Request resolved: facebookincubator#12720 Reviewed By: pedroerp Differential Revision: D71557515 Pulled By: xiaoxmeng fbshipit-source-id: a2faf0c8e76313d118719520378bb7c5a15695cd
Summary: Summary This PR is a split of facebookincubator#12053 to introduce Geometry Presto type in Velox. It focuses on the type declaration without serde code. As suggested, it uses Varbinary as the underlying type. Cast operators can be added later if needed, as Presto Java handles the conversion by functions `ST_GeometryFromText`(`ST_GeomFromBinary`), and `ST_AsText` (`ST_AsBinary`). Pull Request resolved: facebookincubator#12274 Reviewed By: amitkdutta Differential Revision: D71557550 Pulled By: xiaoxmeng fbshipit-source-id: f824a6dcba61df0fee180edc740db124a4a48bc5
Summary: Summary This PR adds Parquet reserved keywords to handle Parquet files generated by the Presto Java workers. Fixes: facebookincubator#12620. References https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/io/parquet/convert/HiveSchemaConverter.java#L154 https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/ParquetHiveSerDe.java#L78 Pull Request resolved: facebookincubator#12625 Reviewed By: pedroerp Differential Revision: D71557476 Pulled By: xiaoxmeng fbshipit-source-id: babeffad086049e9c52cfb16c93a8c2aed1cf029
Summary: Pull Request resolved: facebookincubator#12727 Implement split_to_multimap in velox Reviewed By: zacw7, yuandagits Differential Revision: D71442997 fbshipit-source-id: cd60442d161ad96c70e2c67e859c48aab8dcf165
Summary: Fixes facebookincubator#12526 Pull Request resolved: facebookincubator#12710 Reviewed By: zation99 Differential Revision: D71600558 Pulled By: xiaoxmeng fbshipit-source-id: 37dadf9eced59dcb7fe34105c6e28f82c9e1431c
…acebookincubator#12660) Summary: Prestissimo code using these APIs was advanced in prestodb/presto#24730 Pull Request resolved: facebookincubator#12660 Reviewed By: Yuhta Differential Revision: D71341214 Pulled By: kagamiori fbshipit-source-id: 278c1cbc797a75a030e5611f3ab8dfcc290eb020
Summary: Inherits the gtest test virtually to llow combining test base classes. Allows specifying an extrermal path for LocalRunnerTestBase. Pull Request resolved: facebookincubator#12758 Reviewed By: Yuhta Differential Revision: D71648474 Pulled By: oerling fbshipit-source-id: d17e318fcb6dfa7a2cd459f1a3b5e54ed5cbd6fc
…ebookincubator#12754) Summary: Pull Request resolved: facebookincubator#12754 Some users were incorrectly using PrestoBatchVectorSerializer::serialize which lead to difficult to trace bugs. Adding some simple sanity checks to guard against these. Reviewed By: xiaoxmeng Differential Revision: D71637356 fbshipit-source-id: 5a1f545a3b5f516e02ac52b9a0d08561769d4ef1
…incubator#12756) Summary: Pull Request resolved: facebookincubator#12756 There is a crash in HiveDataSource during the merging of stats because the stats may still be used by the executor in DirectBufferedInput. This means we need the stats to have lifetime exceed that of the executor inside DirectBufferedInput. The simple fix here is to pass a shared_ptr of the stats to DirectBufferedInput so that DirectCoalescedLoad owns a reference to the stats. DwioCoalescedLoadBase was updated to also cache the stats shared_pointer since it can go on the same code path. Reviewed By: Yuhta Differential Revision: D71641158 fbshipit-source-id: cd114c020c88eac3b8c9921b9df41c55f4bdaa28
…incubator#12724) Summary: Pull Request resolved: facebookincubator#12724 Adding custom input generator for phone numbers Reviewed By: duxiao1212 Differential Revision: D71375244 fbshipit-source-id: 5d85de7671c3e592e6fe6b98573640b7ad712351
Summary: The S3 filesystem uses the default logging configuration which creates the file in the current directory where an executable is called from. Most deployments have a custom location to write logs. This change allows the user to specify a path that is used as the new log prefix which results in an absolute path being used for the log file creation. The default AWS log file prefix is used. Pull Request resolved: facebookincubator#12534 Reviewed By: zation99 Differential Revision: D71669762 Pulled By: xiaoxmeng fbshipit-source-id: 727623cf071b10b548f87c48100cad36bfe4aa0b
…bookincubator#12759) Summary: Pull Request resolved: facebookincubator#12759 Add runtime stats to track the wall/cpu times spent in different stages during index lookup processing and integrate with velox operator stats Add metrics to monitor index lookup and table scan batch processing time distribution to ease monitoring an online service Reviewed By: wenqiwooo Differential Revision: D71599185 fbshipit-source-id: 7da229b3ad0726bf6ec7a2f5d0a0dc820f0c5f1d
Summary: Refactor `TaskTraceReader` and consolidate task metadata parsing, We need to extract the node name, plan, configs, etc. from it. Pull Request resolved: facebookincubator#12669 Reviewed By: gggrace14 Differential Revision: D71680632 Pulled By: xiaoxmeng fbshipit-source-id: 0cbe030e12d767931ad39ef2b9df2996c34d11e4
Summary: `-Wunused-private-field` has identified an unused private field. This diff removes it. If the code compiles, this is safe to land. Reviewed By: meyering Differential Revision: D71512007 fbshipit-source-id: 4db078641e7de1e132f32a3dfaa231026aab83e8
…tion (facebookincubator#12765) Summary: Pull Request resolved: facebookincubator#12765 We don't need to copy data for duplicate result for inputs with the same key. Extend row serde to support serialize string view. This optimize cpu/memory and simplify the implementation Also cover the key-dedup in index join test. Reviewed By: wenqiwooo Differential Revision: D71670857 fbshipit-source-id: d3257ef201fe072e8fba0f2931cd3a6101b8210a
…bookincubator#12768) Summary: Pull Request resolved: facebookincubator#12768 Timestamp beyond 2037 is supported now. Fixing a comment. Reviewed By: kgpai Differential Revision: D71689375 fbshipit-source-id: 84eee938c543ad0f7dc497decfd46ff4408c7110
) Summary: When using partition filters with timestamp value, use configuration to control whether to interpret it as local time or UTC. Follow-up for facebookincubator#11957 Pull Request resolved: facebookincubator#12368 Reviewed By: kKPulla Differential Revision: D71047729 Pulled By: kagamiori fbshipit-source-id: 471011591f5c8ff70507d49e425b9749a06b02d4
…facebookincubator#12064) Summary: # PR Description: Detect and address input/output errors that can result in undefined behavior. Ensure proper error handling for I/O functions that may fail and leave variables uninitialized. Neglecting to check the status of these functions before using their outputs (e.g., memory buffers, file descriptors, etc.) can lead to undefined program behavior. This update enforces checks on commonly used I/O functions to validate their return values and prevent improper usage. This is the second PR of the several changes for this refactoring. Pull Request resolved: facebookincubator#12064 Reviewed By: pedroerp Differential Revision: D73005009 Pulled By: kgpai fbshipit-source-id: 5cce140545fb9f682b036ce86875bd2c1b1b5a06
…h + 1 Summary: `-Wambiguous-reversed-operator` warns about ambiguous reversed operators, e.g. `a < b` and `b > a` are both valid. Such operators are disallowed in C++20. This codemod fixes the warnings. #buildsonlynotests - If this diff compiles, it works. - If you approve of this diff, please use the "Accept & Ship" button :-) Reviewed By: dmm-fb Differential Revision: D73131836 fbshipit-source-id: 726d37acc1afd6ffd0c68da95f16eb181ca5b48d
…nds of int32_t (facebookincubator#13048) Summary: Pull Request resolved: facebookincubator#13048 inverse_poisson_cdf doesn't handle the case where results exceed the bounds of int32_t, so it throws an exception (at least with sanitizers enabled) when converting the result to an int32_t. Presto Java handles this by bounding the result to Integer.MAX_VALUE. The change applies the same restriction to Velox's implementation. Reviewed By: skyelves Differential Revision: D73143995 fbshipit-source-id: 9bb2224ae37ccae8e5bf4c8a6f3872882b51469c
…incubator#13049) Summary: Pull Request resolved: facebookincubator#13049 inverse_poisson_cdf has a known issue (coming from the Boost library it invokes) where it can end up in a long running or possibly infinite loop with very large inputs. facebookincubator#13047 Exclude it from fuzzing until this is addressed. Reviewed By: kunigami Differential Revision: D73144904 fbshipit-source-id: 9b6a5032cb07213bf64bf1f8281c3c35a92b1e45
Summary: Pull Request resolved: facebookincubator#13050 Remove unnecessaryTDigest.h sum check. This is too strict and not required Issue: https://www.internalfb.com/tasks/?t=221381362 Reviewed By: spershin Differential Revision: D73147059 fbshipit-source-id: eca749b9528ac2116e9defebb19eea5ab33544ad
…incubator#11612) Summary: Add support for mathematical functions `plus`, `minus`, `multiply`, and `divide` with `IntervalYearMonth` type. The function signatures added match that of [Presto](https://github.com/prestodb/presto/blob/master/presto-main/src/main/java/com/facebook/presto/type/IntervalYearMonthOperators.java), accounting for the Presto function signature changes in prestodb/presto#24089. Pull Request resolved: facebookincubator#11612 Reviewed By: pedroerp Differential Revision: D72997442 Pulled By: kKPulla fbshipit-source-id: 5fcf40639ed0af63e6ec198994aeaef49ba367cb
Summary: [FileSize](https://www.internalfb.com/code/fbsource/[e45d6835b38c]/fbcode/velox/connectors/hive/HiveDataSink.cpp?lines=669) is reported by `IoStatistics` in Velox. [IoStatistics](https://www.internalfb.com/code/fbsource/fbcode/velox/dwio/common/FileSink.cpp?lines=81) is a member of `FileSink`. The `rawBytesWritten` metric is increased per write using FileSink. For DRWF writer and Parquet writer, it uses the [fileSink](https://www.internalfb.com/code/fbsource/[e45d6835b38c467d939c6176cec555e5b53f4a3e]/fbcode/velox/dwio/dwrf/writer/Writer.cpp?lines=157) to write out files. The IO statistics are populated correctly. For SSTable writer, we are using [ZippyDBSstFileWriter](https://www.internalfb.com/code/fbsource/[e45d6835b38c467d939c6176cec555e5b53f4a3e]/fbcode/fb_velox/sst/writer/SSTWriter.cpp?lines=41) which builds another writer using the file name. The write does not go through FileSink. In this diff, I added a new method to get IoStatistics from FileSink. The next diff will use IoStatistics to populate file size. Reviewed By: harsharastogi Differential Revision: D73061995 fbshipit-source-id: cd1399f2cef1e68fb25cf38331db2a3256c86896
…t in the keys (facebookincubator#13039) Summary: Pull Request resolved: facebookincubator#13039 While working on another change I discovered that JoinFuzzer does not test MergeJoin with RIGHT OUTER JOINs. Enabling it locally (I'll publish a change to enable it separately) I discovered a bug. While trying to add a unit test for it, I discovered a few more. They fit into two classes: 1) Skipping over rows on the right side with NULL keys. This is the correct thing to do for INNER and LEFT OUTER JOINs but we need to output misses for these rows in RIGHT OUTER JOINs (they can't hit given our NULL semantics). 2) Writing off the end of the output buffer trying to write out this misses. We need to make sure the size of output_ hasn't yet reached outputBatchSize_ before writing misses to it. This diff fixes the bugs I found and adds unit tests covering NULL keys (I didn't see any prior to this change). Reviewed By: xiaoxmeng Differential Revision: D73077550 fbshipit-source-id: 82d914d38835b51f52676cfa2317fdce164ee0fc
…okincubator#12755) Summary: In this issue, I found that some options in the `WriterOptions` are hardcoded but there are setters for them so it does not make sense that they are not configurable. **In this PR, I mainly made the page size and the batch size configurable.** The page size is configurable in Presto [here](https://github.com/prestodb/presto/blob/master/presto-hive/src/main/java/com/facebook/presto/hive/ParquetFileWriterConfig.java#L49-L59), so we could make it configurable in velox as well. However I think it is not enough to just make the page size configurable, because batch size could also affect the page size since velox parquet writer writes the data in batch, and each batch at most take one page. So if the batch size is too big, the actual page size could be far larger than the page size set by user. So user might also want to set the batch size. Fixes: facebookincubator#12734 Pull Request resolved: facebookincubator#12755 Reviewed By: pedroerp Differential Revision: D72576367 Pulled By: kgpai fbshipit-source-id: e8e3fd3a660c89a47c84e16845d9ea8109076470
Author
|
/ok to test 8ee066e |
Author
|
/ok to test da0be93 |
Author
|
/ok to test 903ccdf |
Author
|
/ok to test 7ed1fc5 |
This reverts commit 7ed1fc5.
Author
|
/ok to test 2926e7e |
VinithKrishnan
pushed a commit
to VinithKrishnan/velox-rapidsai
that referenced
this pull request
Jun 29, 2025
…ger-overflow (facebookincubator#13831) Summary: Pull Request resolved: facebookincubator#13831 This avoids the following errors: ``` fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_abs.h:56:41: runtime error: negation of -9223372036854775808 cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself #0 0x000000346ce5 in std::abs(long) fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_abs.h:56 rapidsai#1 0x000000345879 in std::shared_ptr<facebook::velox::BiasVector<facebook::velox::test::EvalTypeHelper<long>::Type>> facebook::velox::test::VectorMaker::biasVector<long>(std::vector<std::optional<long>, std::allocator<std::optional<long>>> const&) fbcode/velox/vector/tests/utils/VectorMaker-inl.h:58 rapidsai#2 0x000000344d34 in facebook::velox::test::BiasVectorErrorTest::errorTest(std::vector<std::optional<long>, std::allocator<std::optional<long>>>) fbcode/velox/vector/tests/BiasVectorTest.cpp:39 rapidsai#3 0x00000033ec99 in facebook::velox::test::BiasVectorErrorTest_checkRangeTooLargeError_Test::TestBody() fbcode/velox/vector/tests/BiasVectorTest.cpp:44 rapidsai#4 0x7fe0a2342c46 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) fbsource/src/gtest.cc:2727 rapidsai#5 0x7fe0a234275d in testing::Test::Run() fbsource/src/gtest.cc:2744 rapidsai#6 0x7fe0a2345fb3 in testing::TestInfo::Run() fbsource/src/gtest.cc:2890 rapidsai#7 0x7fe0a234c8eb in testing::TestSuite::Run() fbsource/src/gtest.cc:3068 rapidsai#8 0x7fe0a237b52b in testing::internal::UnitTestImpl::RunAllTests() fbsource/src/gtest.cc:6059 rapidsai#9 0x7fe0a237a0a2 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) fbsource/src/gtest.cc:2727 rapidsai#10 0x7fe0a23797f5 in testing::UnitTest::Run() fbsource/src/gtest.cc:5599 rapidsai#11 0x7fe0a2239800 in RUN_ALL_TESTS() fbsource/gtest/gtest.h:2334 rapidsai#12 0x7fe0a223952c in main fbcode/common/gtest/LightMain.cpp:20 rapidsai#13 0x7fe09ec2c656 in __libc_start_call_main /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/nptl/libc_start_call_main.h:58:16 rapidsai#14 0x7fe09ec2c717 in __libc_start_main@GLIBC_2.2.5 /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../csu/libc-start.c:409:3 rapidsai#15 0x00000033d8b0 in _start /home/engshare/third-party2/glibc/2.34/src/glibc-2.34/csu/../sysdeps/x86_64/start.S:116 UndefinedBehaviorSanitizer: signed-integer-overflow fbcode/third-party-buck/platform010/build/libgcc/include/c++/trunk/bits/std_abs.h:56:41 ``` Avoid overflow by using the expression (static_cast<uint64_t>(1) + ~static_cast<uint64_t>(min)) to calculate the absolute value of min without using std::abs Reviewed By: dmm-fb, peterenescu Differential Revision: D76901449 fbshipit-source-id: 7eb3bd0f83e42f44cdf34ea1759f3aa9e1042dae
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
Updates from Velox main, now including first upstream PR with OrderBy.
Changes: