From 404aa77849f76584f31d0b5be0d77b109e8e5253 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Mon, 11 Sep 2023 15:13:50 -0400 Subject: [PATCH 01/16] Fix a typo in the data_stream _stats API documentation (#99438) --- docs/reference/indices/data-stream-stats.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/indices/data-stream-stats.asciidoc b/docs/reference/indices/data-stream-stats.asciidoc index 3d27eacf830da..d35e2738d0321 100644 --- a/docs/reference/indices/data-stream-stats.asciidoc +++ b/docs/reference/indices/data-stream-stats.asciidoc @@ -120,7 +120,7 @@ Total number of selected data streams. (integer) Total number of backing indices for the selected data streams. -`total_store_sizes`:: +`total_store_size`:: (<>) Total size of all shards for the selected data streams. This property is included only if the `human` query parameter is `true`. From a7617db23d118cf9977edebfefeab74a8c7127da Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 11 Sep 2023 15:19:43 -0400 Subject: [PATCH 02/16] Drop changelog (#99436) We don't want it. --- docs/changelog/99434.yaml | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 docs/changelog/99434.yaml diff --git a/docs/changelog/99434.yaml b/docs/changelog/99434.yaml deleted file mode 100644 index b03bc4f3c9b41..0000000000000 --- a/docs/changelog/99434.yaml +++ /dev/null @@ -1,5 +0,0 @@ -pr: 99434 -summary: "ESQL: Disable optimizations with bad null handling" -area: ES|QL -type: bug -issues: [] From 6f90fd7ecfc111af5753cf6483d578abd2a28a5a Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 11 Sep 2023 16:18:48 -0400 Subject: [PATCH 03/16] [buildkite] Add buildkite pr-bot config for triggering builds with opt-in label (#99446) --- .buildkite/pull-requests.json | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 .buildkite/pull-requests.json diff --git a/.buildkite/pull-requests.json b/.buildkite/pull-requests.json new file mode 100644 index 0000000000000..c6682e55642c5 --- /dev/null +++ b/.buildkite/pull-requests.json @@ -0,0 +1,18 @@ +{ + "jobs": [ + { + "enabled": true, + "pipeline_slug": "elasticsearch-pull-request", + "allow_org_users": true, + "allowed_repo_permissions": [ + "admin", + "write" + ], + "set_commit_status": false, + "build_on_commit": true, + "build_on_comment": true, + "trigger_comment_regex": "buildkite\\W+elasticsearch-ci.+", + "labels": "buildkite-opt-in" + } + ] +} From 590f27a5cbc8c75ba321fdacab625e30774aeba3 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 11 Sep 2023 16:25:43 -0400 Subject: [PATCH 04/16] Fix pull-requests.json --- .buildkite/pull-requests.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.buildkite/pull-requests.json b/.buildkite/pull-requests.json index c6682e55642c5..466b69e008241 100644 --- a/.buildkite/pull-requests.json +++ b/.buildkite/pull-requests.json @@ -12,7 +12,7 @@ "build_on_commit": true, "build_on_comment": true, "trigger_comment_regex": "buildkite\\W+elasticsearch-ci.+", - "labels": "buildkite-opt-in" + "labels": ["buildkite-opt-in"] } ] } From 6602b6c726daee6ccf48a86e2208c1a1c9bc7a24 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 12 Sep 2023 00:22:11 +0300 Subject: [PATCH 05/16] ESQL: create a Vector when needed for IN (#99382) --- docs/changelog/99382.yaml | 6 ++++++ .../qa/testFixtures/src/main/resources/string.csv-spec | 7 +++++++ .../evaluator/predicate/operator/comparison/InMapper.java | 7 ++++++- 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/99382.yaml diff --git a/docs/changelog/99382.yaml b/docs/changelog/99382.yaml new file mode 100644 index 0000000000000..5f5eb932ed458 --- /dev/null +++ b/docs/changelog/99382.yaml @@ -0,0 +1,6 @@ +pr: 99382 +summary: "ESQL: create a Vector when needed for IN" +area: ES|QL +type: bug +issues: + - 99347 diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec index aa893e63e1a30..357f6369dca73 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/string.csv-spec @@ -327,6 +327,13 @@ emp_no:integer |job_positions:keyword |is_in:boolean 10026 |Reporting Analyst |null ; +in3VLWithNull-99347_bugfix +from employees | where emp_no == 10025 | keep emp_no, job_positions | eval is_in = job_positions in ("Accountant", "Internship", null); + +emp_no:integer |job_positions:keyword |is_in:boolean +10025 |Accountant |true +; + in3VLWithComputedNull from employees | sort emp_no | where mv_count(job_positions) <= 1 | where emp_no >= 10024 | limit 3 | keep emp_no, job_positions | eval nil = concat("", null) | eval is_in = job_positions in ("Accountant", "Internship", nil); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/InMapper.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/InMapper.java index 7ed08b658c75e..b99cccab54ba3 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/InMapper.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/predicate/operator/comparison/InMapper.java @@ -106,7 +106,12 @@ private static Block evalWithNulls(boolean[] values, BitSet nulls, boolean nullI nulls.set(i); } // else: leave nulls as is } - return new BooleanArrayBlock(values, values.length, null, nulls, Block.MvOrdering.UNORDERED); + if (nulls.isEmpty()) { + // no nulls and no multi-values means we must use a Vector + return new BooleanArrayVector(values, values.length).asBlock(); + } else { + return new BooleanArrayBlock(values, values.length, null, nulls, Block.MvOrdering.UNORDERED); + } } } } From f241f2bb6ae6944799fe6cf36d5674a0ec46667f Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 12 Sep 2023 01:22:27 +0300 Subject: [PATCH 06/16] ESQL: Make the stats test more deterministic (for multi-node testing) (#99451) --- .../src/main/resources/stats.csv-spec | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec index c1f623fda251d..a8918251b5ed2 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec @@ -360,21 +360,21 @@ byUnmentionedLongAndLong FROM employees | EVAL trunk_worked_seconds = avg_worked_seconds / 100000000 * 100000000 | STATS c = count(gender) by languages.long, trunk_worked_seconds -| SORT c desc, trunk_worked_seconds; +| SORT c desc, trunk_worked_seconds, languages.long; c:long | languages.long:long | trunk_worked_seconds:long -13 |5 |300000000 -10 |2 |300000000 - 9 |3 |200000000 - 9 |4 |300000000 - 8 |4 |200000000 - 8 |3 |300000000 - 7 |1 |200000000 - 6 |2 |200000000 - 6 |null |300000000 - 6 |1 |300000000 - 4 |null |200000000 - 4 |5 |200000000 +13 |5 |300000000 +10 |2 |300000000 +9 |3 |200000000 +9 |4 |300000000 +8 |4 |200000000 +8 |3 |300000000 +7 |1 |200000000 +6 |2 |200000000 +6 |1 |300000000 +6 |null |300000000 +4 |5 |200000000 +4 |null |200000000 ; byUnmentionedIntAndLong From 4ee229779b8a448953a97402401be81f0455d3d8 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 12 Sep 2023 07:16:39 +0100 Subject: [PATCH 07/16] Clean up delete code in S3BlobContainer (#99447) Simplifies things using utils from `Iterators` that didn't exist when the code was first written. --- .../repositories/s3/S3BlobContainer.java | 55 +++---------------- .../repositories/s3/S3BlobStore.java | 4 ++ 2 files changed, 13 insertions(+), 46 deletions(-) diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java index 0057f36d94cb8..86650bc0fe9c2 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobContainer.java @@ -23,7 +23,6 @@ import com.amazonaws.services.s3.model.ObjectMetadata; import com.amazonaws.services.s3.model.PartETag; import com.amazonaws.services.s3.model.PutObjectRequest; -import com.amazonaws.services.s3.model.S3ObjectSummary; import com.amazonaws.services.s3.model.UploadPartRequest; import com.amazonaws.services.s3.model.UploadPartResult; @@ -60,7 +59,6 @@ import java.io.OutputStream; import java.time.Instant; import java.util.ArrayList; -import java.util.Collections; import java.util.Date; import java.util.Iterator; import java.util.List; @@ -272,14 +270,13 @@ public void writeBlobAtomic(String blobName, BytesReference bytes, boolean failI } @Override - @SuppressWarnings("unchecked") public DeleteResult delete() throws IOException { final AtomicLong deletedBlobs = new AtomicLong(); final AtomicLong deletedBytes = new AtomicLong(); try (AmazonS3Reference clientReference = blobStore.clientReference()) { ObjectListing prevListing = null; while (true) { - ObjectListing list; + final ObjectListing list; if (prevListing != null) { final var listNextBatchOfObjectsRequest = new ListNextBatchOfObjectsRequest(prevListing); listNextBatchOfObjectsRequest.setRequestMetricCollector(blobStore.listMetricCollector); @@ -291,26 +288,16 @@ public DeleteResult delete() throws IOException { listObjectsRequest.setRequestMetricCollector(blobStore.listMetricCollector); list = SocketAccess.doPrivileged(() -> clientReference.client().listObjects(listObjectsRequest)); } - final Iterator objectSummaryIterator = list.getObjectSummaries().iterator(); - final Iterator blobNameIterator = new Iterator<>() { - @Override - public boolean hasNext() { - return objectSummaryIterator.hasNext(); - } - - @Override - public String next() { - final S3ObjectSummary summary = objectSummaryIterator.next(); - deletedBlobs.incrementAndGet(); - deletedBytes.addAndGet(summary.getSize()); - return summary.getKey(); - } - }; + final Iterator blobNameIterator = Iterators.map(list.getObjectSummaries().iterator(), summary -> { + deletedBlobs.incrementAndGet(); + deletedBytes.addAndGet(summary.getSize()); + return summary.getKey(); + }); if (list.isTruncated()) { - doDeleteBlobs(blobNameIterator, false); + blobStore.deleteBlobsIgnoringIfNotExists(blobNameIterator); prevListing = list; } else { - doDeleteBlobs(Iterators.concat(blobNameIterator, Collections.singletonList(keyPath).iterator()), false); + blobStore.deleteBlobsIgnoringIfNotExists(Iterators.concat(blobNameIterator, Iterators.single(keyPath))); break; } } @@ -322,31 +309,7 @@ public String next() { @Override public void deleteBlobsIgnoringIfNotExists(Iterator blobNames) throws IOException { - doDeleteBlobs(blobNames, true); - } - - private void doDeleteBlobs(Iterator blobNames, boolean relative) throws IOException { - if (blobNames.hasNext() == false) { - return; - } - final Iterator outstanding; - if (relative) { - outstanding = new Iterator<>() { - @Override - public boolean hasNext() { - return blobNames.hasNext(); - } - - @Override - public String next() { - return buildKey(blobNames.next()); - } - }; - } else { - outstanding = blobNames; - } - - blobStore.deleteBlobsIgnoringIfNotExists(outstanding); + blobStore.deleteBlobsIgnoringIfNotExists(Iterators.map(blobNames, this::buildKey)); } @Override diff --git a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java index f25ee58772859..027fd03d83c55 100644 --- a/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java +++ b/modules/repository-s3/src/main/java/org/elasticsearch/repositories/s3/S3BlobStore.java @@ -215,6 +215,10 @@ public BlobContainer blobContainer(BlobPath path) { @Override public void deleteBlobsIgnoringIfNotExists(Iterator blobNames) throws IOException { + if (blobNames.hasNext() == false) { + return; + } + final List partition = new ArrayList<>(); try (AmazonS3Reference clientReference = clientReference()) { // S3 API only allows 1k blobs per delete so we split up the given blobs into requests of max. 1k deletes From 4c8888de9e8e0da25369a81457d33e255cf8c993 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 12 Sep 2023 18:08:05 +1000 Subject: [PATCH 08/16] Do not report failure after connections are made (#99117) Today, when the number of attempts is exhausted, ProxyConnectionStrategy checks the number of connections before returns. It reports connection failure if the number of connections is zero at the time of checking. However, this behaviour is incorrect. In rare cases, a connection can be dropped right after it is initially established and before the number checking. From the perspective of the `openConnections` method, it should not care whether or when opened connections are subsequently closed. As long as connections have been initially established, it should report success instead of failure. This PR adjusts the code to report success in above situation. Relates: #94998 Resolves: #99113 --- docs/changelog/99117.yaml | 5 ++ .../transport/ProxyConnectionStrategy.java | 21 +++----- .../ProxyConnectionStrategyTests.java | 51 +++++++++++++++++++ 3 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 docs/changelog/99117.yaml diff --git a/docs/changelog/99117.yaml b/docs/changelog/99117.yaml new file mode 100644 index 0000000000000..491692f232081 --- /dev/null +++ b/docs/changelog/99117.yaml @@ -0,0 +1,5 @@ +pr: 99117 +summary: Do not report failure after connections are made +area: Network +type: bug +issues: [] diff --git a/server/src/main/java/org/elasticsearch/transport/ProxyConnectionStrategy.java b/server/src/main/java/org/elasticsearch/transport/ProxyConnectionStrategy.java index 83a0860ba6324..35655f6260461 100644 --- a/server/src/main/java/org/elasticsearch/transport/ProxyConnectionStrategy.java +++ b/server/src/main/java/org/elasticsearch/transport/ProxyConnectionStrategy.java @@ -32,7 +32,6 @@ import java.util.Collections; import java.util.Map; import java.util.Objects; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -315,19 +314,13 @@ public void onFailure(Exception e) { })); } } else { - int openConnections = connectionManager.size(); - if (openConnections == 0) { - assert false : "should not happen since onFailure should catch it and report with underlying cause"; - finished.onFailure(getNoSeedNodeLeftException(Set.of())); - } else { - logger.debug( - "unable to open maximum number of connections [remote cluster: {}, opened: {}, maximum: {}]", - clusterAlias, - openConnections, - maxNumConnections - ); - finished.onResponse(null); - } + logger.debug( + "unable to open maximum number of connections [remote cluster: {}, opened: {}, maximum: {}]", + clusterAlias, + connectionManager.size(), + maxNumConnections + ); + finished.onResponse(null); } } diff --git a/server/src/test/java/org/elasticsearch/transport/ProxyConnectionStrategyTests.java b/server/src/test/java/org/elasticsearch/transport/ProxyConnectionStrategyTests.java index d2941bab3f91a..965288a989870 100644 --- a/server/src/test/java/org/elasticsearch/transport/ProxyConnectionStrategyTests.java +++ b/server/src/test/java/org/elasticsearch/transport/ProxyConnectionStrategyTests.java @@ -43,8 +43,10 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItemInArray; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.spy; @@ -480,6 +482,55 @@ public void testProxyStrategyWillResolveAddressesEachConnect() throws Exception } } + public void testConnectionsClosedAfterInitiallyEstablishedDoesNotLeadToFailure() throws InterruptedException { + try (MockTransportService remoteService = startTransport("proxy_node", VersionInformation.CURRENT, TransportVersion.current())) { + TransportAddress address = remoteService.boundAddress().publishAddress(); + + try ( + MockTransportService localService = MockTransportService.createNewService( + Settings.EMPTY, + VersionInformation.CURRENT, + TransportVersion.current(), + threadPool + ) + ) { + localService.start(); + + final var connectionManager = new ClusterConnectionManager(profile, localService.transport, threadPool.getThreadContext()); + final int numOfConnections = randomIntBetween(4, 8); + final var connectionCountDown = new CountDownLatch(numOfConnections); + connectionManager.addListener(new TransportConnectionListener() { + @Override + public void onNodeConnected(DiscoveryNode node, Transport.Connection connection) { + // Count down to ensure at least the required number of connection are indeed initially established + connectionCountDown.countDown(); + // Simulate disconnection right after connection is made + connection.close(); + } + }); + + try ( + var remoteConnectionManager = new RemoteConnectionManager(clusterAlias, connectionManager); + var strategy = new ProxyConnectionStrategy( + clusterAlias, + localService, + remoteConnectionManager, + Settings.EMPTY, + numOfConnections, + address.toString() + ) + ) { + final PlainActionFuture connectFuture = PlainActionFuture.newFuture(); + strategy.connect(connectFuture); + // Should see no error and the connection size is 0 + connectFuture.actionGet(); + assertThat(connectionCountDown.await(30L, TimeUnit.SECONDS), is(true)); + assertThat(remoteConnectionManager.size(), equalTo(0)); + } + } + } + } + public void testProxyStrategyWillNeedToBeRebuiltIfNumOfSocketsOrAddressesOrServerNameChange() { try (MockTransportService remoteTransport = startTransport("node1", VersionInformation.CURRENT, TransportVersion.current())) { TransportAddress remoteAddress = remoteTransport.boundAddress().publishAddress(); From ae2aacb66affa098f1e431a296a0dc4898ccd640 Mon Sep 17 00:00:00 2001 From: Kostas Krikellas <131142368+kkrik-es@users.noreply.github.com> Date: Tue, 12 Sep 2023 11:54:54 +0300 Subject: [PATCH 09/16] Disable FilterByFilterAggregator through ClusterSettings (#99417) `search.aggs.rewrite_to_filter_by_filter` allows disabling FilterByFilterAggregator when used in terms and range aggregation. The same should apply to filter aggregation. Fixes #99335 --- docs/changelog/99417.yaml | 6 ++++++ .../bucket/filter/FilterByFilterAggregator.java | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/99417.yaml diff --git a/docs/changelog/99417.yaml b/docs/changelog/99417.yaml new file mode 100644 index 0000000000000..8c88a5a548dff --- /dev/null +++ b/docs/changelog/99417.yaml @@ -0,0 +1,6 @@ +pr: 99417 +summary: Disable `FilterByFilterAggregator` through `ClusterSettings` +area: Aggregations +type: enhancement +issues: + - 99335 diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterByFilterAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterByFilterAggregator.java index a2ce68e3fc29e..3a2cce587f34f 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterByFilterAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterByFilterAggregator.java @@ -134,7 +134,7 @@ final void add(QueryToFilterAdapter filter) throws IOException { * Build the the adapter or {@code null} if the this isn't a valid rewrite. */ public final T build() throws IOException { - if (false == valid) { + if (false == valid || aggCtx.enableRewriteToFilterByFilter() == false) { return null; } class AdapterBuild implements CheckedFunction { From 16a4e542f09acef5e69daf41f5431bf2c4096dc2 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 12 Sep 2023 19:24:45 +1000 Subject: [PATCH 10/16] [Test] More robust order of assertions (#99461) This PR adjusts the order of assertions to ensure we are done with the atomic reference variable in the previous assertions before changing it to null. Resolves: #99406 --- ...tty4ServerTransportAuthenticationTests.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4ServerTransportAuthenticationTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4ServerTransportAuthenticationTests.java index 2a15aa09ddccd..99a411ab11a90 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4ServerTransportAuthenticationTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/netty4/SecurityNetty4ServerTransportAuthenticationTests.java @@ -204,9 +204,12 @@ public void testProxyStrategyConnectionClosesWhenAuthenticatorAlwaysFails() thro fail("No connection should be available if authn fails"); }, e -> { logger.info("Expected: no connection could not be established"); - connectionTestDone.countDown(); - assertThat(e, instanceOf(RemoteTransportException.class)); - assertThat(e.getCause(), instanceOf(authenticationException.get().getClass())); + try { + assertThat(e, instanceOf(RemoteTransportException.class)); + assertThat(e.getCause(), instanceOf(authenticationException.get().getClass())); + } finally { + connectionTestDone.countDown(); + } })); assertTrue(connectionTestDone.await(10L, TimeUnit.SECONDS)); } @@ -261,9 +264,12 @@ public void testSniffStrategyNoConnectionWhenAuthenticatorAlwaysFails() throws E fail("No connection should be available if authn fails"); }, e -> { logger.info("Expected: no connection could be established"); - connectionTestDone.countDown(); - assertThat(e, instanceOf(RemoteTransportException.class)); - assertThat(e.getCause(), instanceOf(authenticationException.get().getClass())); + try { + assertThat(e, instanceOf(RemoteTransportException.class)); + assertThat(e.getCause(), instanceOf(authenticationException.get().getClass())); + } finally { + connectionTestDone.countDown(); + } })); assertTrue(connectionTestDone.await(10L, TimeUnit.SECONDS)); } From 22a2cd0a0205745e050b973d28c58186f5f6be58 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 12 Sep 2023 14:04:58 +0300 Subject: [PATCH 11/16] More deterministic tests (#99469) --- .../qa/testFixtures/src/main/resources/stats.csv-spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec index a8918251b5ed2..da559485d17ff 100644 --- a/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec +++ b/x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec @@ -293,15 +293,15 @@ byStringAndLong FROM employees | EVAL trunk_worked_seconds = avg_worked_seconds / 100000000 * 100000000 | STATS c = COUNT(gender) by gender, trunk_worked_seconds -| SORT c desc; +| SORT c desc, gender, trunk_worked_seconds desc; c:long | gender:keyword | trunk_worked_seconds:long 30 | M | 300000000 27 | M | 200000000 22 | F | 300000000 11 | F | 200000000 - 0 | null | 200000000 0 | null | 300000000 + 0 | null | 200000000 ; byStringAndLongWithAlias @@ -310,15 +310,15 @@ FROM employees | RENAME gender as g, trunk_worked_seconds as tws | KEEP g, tws | STATS c = count(g) by g, tws -| SORT c desc; +| SORT c desc, g, tws desc; c:long | g:keyword | tws:long 30 | M | 300000000 27 | M | 200000000 22 | F | 300000000 11 | F | 200000000 - 0 | null | 200000000 0 | null | 300000000 + 0 | null | 200000000 ; byStringAndString From 30f6e51804905c12d2b302ed9334bf5b5c7feae6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Krze=C5=9Bniak?= Date: Tue, 12 Sep 2023 13:13:38 +0200 Subject: [PATCH 12/16] Update ES|QL (#99467) To make it more clear let's use different index names for comma-separated index list --- docs/reference/esql/source-commands/from.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/reference/esql/source-commands/from.asciidoc b/docs/reference/esql/source-commands/from.asciidoc index 69ab152de9cd8..2a83f3abc8a4e 100644 --- a/docs/reference/esql/source-commands/from.asciidoc +++ b/docs/reference/esql/source-commands/from.asciidoc @@ -25,7 +25,7 @@ or aliases: [source,esql] ---- -FROM employees-00001,employees-* +FROM employees-00001,other-employees-* ---- Use the `METADATA` directive to enable <>: From 54f6e4f51bbbf851c3f7e6fabed977d26ddebd52 Mon Sep 17 00:00:00 2001 From: Abdon Pijpelink Date: Tue, 12 Sep 2023 13:25:56 +0200 Subject: [PATCH 13/16] [DOCS] Remove 'coming in 8.10' from remote cluster API key auth docs (#99462) --- docs/reference/modules/cluster/remote-clusters-api-key.asciidoc | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/reference/modules/cluster/remote-clusters-api-key.asciidoc b/docs/reference/modules/cluster/remote-clusters-api-key.asciidoc index 29fe2b0aaf35e..9451c8ba50aae 100644 --- a/docs/reference/modules/cluster/remote-clusters-api-key.asciidoc +++ b/docs/reference/modules/cluster/remote-clusters-api-key.asciidoc @@ -1,7 +1,6 @@ [[remote-clusters-api-key]] === Add remote clusters using API key authentication -coming::[8.10] beta::[] API key authentication enables a local cluster to authenticate itself with a From 403bcb366a218e749916f9a11013ac0b0454e3d6 Mon Sep 17 00:00:00 2001 From: Milton Hultgren Date: Tue, 12 Sep 2023 14:11:30 +0200 Subject: [PATCH 14/16] Update CODEOWNER paths for Stack Monitoring mappings (#99428) --- .github/CODEOWNERS | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index c29ed92f39547..cbe2d8ba4a82d 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -3,17 +3,17 @@ # For more info, see https://help.github.com/articles/about-codeowners/ # Stack Monitoring index templates -x-pack/plugin/core/src/main/resources/monitoring-alerts-7.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-beats-mb.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-beats.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-ent-search-mb.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-es-mb.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-es.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-kibana-mb.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-kibana.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-logstash-mb.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-logstash.json @elastic/infra-monitoring-ui -x-pack/plugin/core/src/main/resources/monitoring-mb-ilm-policy.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-alerts-7.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-beats-mb.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-beats.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-ent-search-mb.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-es-mb.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-es.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-kibana-mb.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-kibana.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-logstash-mb.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-logstash.json @elastic/infra-monitoring-ui +x-pack/plugin/core/template-resources/src/main/resources/monitoring-mb-ilm-policy.json @elastic/infra-monitoring-ui x-pack/plugin/monitoring/src/main/java/org/elasticsearch/xpack/monitoring/MonitoringTemplateRegistry.java @elastic/infra-monitoring-ui # Fleet From e26dca469d27d94eed8812c52d43e2961e4eba2d Mon Sep 17 00:00:00 2001 From: Mark Tozzi Date: Tue, 12 Sep 2023 08:12:20 -0400 Subject: [PATCH 15/16] [ESQL] Plumb through ranges and warnings for the casting to double tests (#99452) Add the warnings and range checking parameters to unary and binary casting to double test generators. I also moved the data type to the value supplier, which the binary case needed. That feels more right - that's what I was intending with TypedData to begin with, but our abstractions are still messy here. --- .../expression/function/TestCaseSupplier.java | 325 ++++++++++-------- .../scalar/convert/ToVersionTests.java | 7 +- .../function/scalar/math/AcosTests.java | 3 +- .../function/scalar/math/AsinTests.java | 3 +- .../function/scalar/math/Atan2Tests.java | 12 +- .../function/scalar/math/AtanTests.java | 3 +- .../function/scalar/math/CosTests.java | 3 +- .../function/scalar/math/CoshTests.java | 3 +- .../function/scalar/math/SinTests.java | 3 +- .../function/scalar/math/SinhTests.java | 3 +- .../function/scalar/math/TanTests.java | 3 +- .../function/scalar/math/TanhTests.java | 3 +- 12 files changed, 208 insertions(+), 163 deletions(-) diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java index 3b6174bac5bf4..8b113bb12d605 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/TestCaseSupplier.java @@ -26,7 +26,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Map; import java.util.function.DoubleBinaryOperator; import java.util.function.DoubleFunction; import java.util.function.DoubleUnaryOperator; @@ -94,7 +93,8 @@ public static List forUnaryCastingToDouble( String argName, DoubleUnaryOperator expected, Double min, - Double max + Double max, + List warnings ) { String read = "Attribute[channel=0]"; String eval = name + "[" + argName + "="; @@ -106,7 +106,7 @@ public static List forUnaryCastingToDouble( i -> expected.applyAsDouble(i), min.intValue(), max.intValue(), - List.of() + warnings ); forUnaryLong( suppliers, @@ -115,7 +115,7 @@ public static List forUnaryCastingToDouble( l -> expected.applyAsDouble(l), min.longValue(), max.longValue(), - List.of() + warnings ); forUnaryUnsignedLong( suppliers, @@ -124,9 +124,9 @@ public static List forUnaryCastingToDouble( ul -> expected.applyAsDouble(ul.doubleValue()), BigInteger.valueOf((int) Math.ceil(min)), BigInteger.valueOf((int) Math.floor(max)), - List.of() + warnings ); - forUnaryDouble(suppliers, eval + read + "]", DataTypes.DOUBLE, i -> expected.applyAsDouble(i), min, max, List.of()); + forUnaryDouble(suppliers, eval + read + "]", DataTypes.DOUBLE, i -> expected.applyAsDouble(i), min, max, warnings); return suppliers; } @@ -138,47 +138,60 @@ public static List forBinaryCastingToDouble( String name, String lhsName, String rhsName, - DoubleBinaryOperator expected + DoubleBinaryOperator expected, + Double lhsMin, + Double lhsMax, + Double rhsMin, + Double rhsMax, + List warnings ) { List suppliers = new ArrayList<>(); - for (DataType lhsType : EsqlDataTypes.types()) { - if (lhsType.isNumeric() == false || EsqlDataTypes.isRepresentable(lhsType) == false) { - continue; - } - for (Map.Entry> lhsSupplier : RANDOM_VALUE_SUPPLIERS.get(lhsType)) { - for (DataType rhsType : EsqlDataTypes.types()) { - if (rhsType.isNumeric() == false || EsqlDataTypes.isRepresentable(rhsType) == false) { - continue; + List lhsSuppliers = new ArrayList<>(); + List rhsSuppliers = new ArrayList<>(); + + lhsSuppliers.addAll(intCases(lhsMin.intValue(), lhsMax.intValue())); + lhsSuppliers.addAll(longCases(lhsMin.longValue(), lhsMax.longValue())); + lhsSuppliers.addAll(ulongCases(BigInteger.valueOf((long) Math.ceil(lhsMin)), BigInteger.valueOf((long) Math.floor(lhsMax)))); + lhsSuppliers.addAll(doubleCases(lhsMin, lhsMax)); + + rhsSuppliers.addAll(intCases(rhsMin.intValue(), rhsMax.intValue())); + rhsSuppliers.addAll(longCases(rhsMin.longValue(), rhsMax.longValue())); + rhsSuppliers.addAll(ulongCases(BigInteger.valueOf((long) Math.ceil(rhsMin)), BigInteger.valueOf((long) Math.floor(rhsMax)))); + rhsSuppliers.addAll(doubleCases(rhsMin, rhsMax)); + + for (TypedDataSupplier lhsSupplier : lhsSuppliers) { + for (TypedDataSupplier rhsSupplier : rhsSuppliers) { + String caseName = lhsSupplier.name() + ", " + rhsSupplier.name(); + suppliers.add(new TestCaseSupplier(caseName, List.of(lhsSupplier.type(), rhsSupplier.type()), () -> { + Number lhs = (Number) lhsSupplier.supplier().get(); + Number rhs = (Number) rhsSupplier.supplier().get(); + TypedData lhsTyped = new TypedData( + // TODO there has to be a better way to handle unsigned long + lhs instanceof BigInteger b ? NumericUtils.asLongUnsigned(b) : lhs, + lhsSupplier.type(), + "lhs" + ); + TypedData rhsTyped = new TypedData( + rhs instanceof BigInteger b ? NumericUtils.asLongUnsigned(b) : rhs, + rhsSupplier.type(), + "rhs" + ); + String lhsEvalName = castToDoubleEvaluator("Attribute[channel=0]", lhsSupplier.type()); + String rhsEvalName = castToDoubleEvaluator("Attribute[channel=1]", rhsSupplier.type()); + TestCase testCase = new TestCase( + List.of(lhsTyped, rhsTyped), + name + "[" + lhsName + "=" + lhsEvalName + ", " + rhsName + "=" + rhsEvalName + "]", + DataTypes.DOUBLE, + equalTo(expected.applyAsDouble(lhs.doubleValue(), rhs.doubleValue())) + ); + for (String warning : warnings) { + testCase = testCase.withWarning(warning); } - for (Map.Entry> rhsSupplier : RANDOM_VALUE_SUPPLIERS.get(rhsType)) { - String caseName = lhsSupplier.getKey() + ", " + rhsSupplier.getKey(); - suppliers.add(new TestCaseSupplier(caseName, List.of(lhsType, rhsType), () -> { - Number lhs = (Number) lhsSupplier.getValue().get(); - Number rhs = (Number) rhsSupplier.getValue().get(); - TypedData lhsTyped = new TypedData( - // TODO there has to be a better way to handle unsigned long - lhs instanceof BigInteger b ? NumericUtils.asLongUnsigned(b) : lhs, - lhsType, - "lhs" - ); - TypedData rhsTyped = new TypedData( - rhs instanceof BigInteger b ? NumericUtils.asLongUnsigned(b) : rhs, - rhsType, - "rhs" - ); - String lhsEvalName = castToDoubleEvaluator("Attribute[channel=0]", lhsType); - String rhsEvalName = castToDoubleEvaluator("Attribute[channel=1]", rhsType); - return new TestCase( - List.of(lhsTyped, rhsTyped), - name + "[" + lhsName + "=" + lhsEvalName + ", " + rhsName + "=" + rhsEvalName + "]", - DataTypes.DOUBLE, - equalTo(expected.applyAsDouble(lhs.doubleValue(), rhs.doubleValue())) - ); - })); - } - } + return testCase; + })); } } + return suppliers; } @@ -352,7 +365,7 @@ public static void forUnaryStrings( suppliers, expectedEvaluatorToString, type, - stringCases(type.typeName()), + stringCases(type), expectedType, v -> expectedValue.apply((BytesRef) v), warnings @@ -385,7 +398,7 @@ private static void unaryNumeric( List suppliers, String expectedEvaluatorToString, DataType inputType, - List>> valueSuppliers, + List valueSuppliers, DataType expectedOutputType, Function expected, List warnings @@ -405,18 +418,18 @@ private static void unary( List suppliers, String expectedEvaluatorToString, DataType inputType, - List>> valueSuppliers, + List valueSuppliers, DataType expectedOutputType, Function expected, List warnings ) { - for (Map.Entry> supplier : valueSuppliers) { - suppliers.add(new TestCaseSupplier(supplier.getKey(), List.of(inputType), () -> { - Object value = supplier.getValue().get(); + for (TypedDataSupplier supplier : valueSuppliers) { + suppliers.add(new TestCaseSupplier(supplier.name(), List.of(supplier.type()), () -> { + Object value = supplier.supplier().get(); TypedData typed = new TypedData( // TODO there has to be a better way to handle unsigned long value instanceof BigInteger b ? NumericUtils.asLongUnsigned(b) : value, - inputType, + supplier.type(), "value" ); TestCase testCase = new TestCase( @@ -433,61 +446,61 @@ private static void unary( } } - private static List>> intCases(int min, int max) { - List>> cases = new ArrayList<>(); + private static List intCases(int min, int max) { + List cases = new ArrayList<>(); if (0 <= max && 0 >= min) { - cases.add(Map.entry("<0 int>", () -> 0)); + cases.add(new TypedDataSupplier("<0 int>", () -> 0, DataTypes.INTEGER)); } int lower = Math.max(min, 1); int upper = Math.min(max, Integer.MAX_VALUE); if (lower < upper) { - cases.add(Map.entry("", () -> ESTestCase.randomIntBetween(lower, upper))); + cases.add(new TypedDataSupplier("", () -> ESTestCase.randomIntBetween(lower, upper), DataTypes.INTEGER)); } else if (lower == upper) { - cases.add(Map.entry("<" + lower + " int>", () -> lower)); + cases.add(new TypedDataSupplier("<" + lower + " int>", () -> lower, DataTypes.INTEGER)); } int lower1 = Math.max(min, Integer.MIN_VALUE); int upper1 = Math.min(max, -1); if (lower1 < upper1) { - cases.add(Map.entry("", () -> ESTestCase.randomIntBetween(lower1, upper1))); + cases.add(new TypedDataSupplier("", () -> ESTestCase.randomIntBetween(lower1, upper1), DataTypes.INTEGER)); } else if (lower1 == upper1) { - cases.add(Map.entry("<" + lower1 + " int>", () -> lower1)); + cases.add(new TypedDataSupplier("<" + lower1 + " int>", () -> lower1, DataTypes.INTEGER)); } return cases; } - private static List>> longCases(long min, long max) { - List>> cases = new ArrayList<>(); + private static List longCases(long min, long max) { + List cases = new ArrayList<>(); if (0L <= max && 0L >= min) { - cases.add(Map.entry("<0 long>", () -> 0L)); + cases.add(new TypedDataSupplier("<0 long>", () -> 0L, DataTypes.LONG)); } long lower = Math.max(min, 1); long upper = Math.min(max, Long.MAX_VALUE); if (lower < upper) { - cases.add(Map.entry("", () -> ESTestCase.randomLongBetween(lower, upper))); + cases.add(new TypedDataSupplier("", () -> ESTestCase.randomLongBetween(lower, upper), DataTypes.LONG)); } else if (lower == upper) { - cases.add(Map.entry("<" + lower + " long>", () -> lower)); + cases.add(new TypedDataSupplier("<" + lower + " long>", () -> lower, DataTypes.LONG)); } long lower1 = Math.max(min, Long.MIN_VALUE); long upper1 = Math.min(max, -1); if (lower1 < upper1) { - cases.add(Map.entry("", () -> ESTestCase.randomLongBetween(lower1, upper1))); + cases.add(new TypedDataSupplier("", () -> ESTestCase.randomLongBetween(lower1, upper1), DataTypes.LONG)); } else if (lower1 == upper1) { - cases.add(Map.entry("<" + lower1 + " long>", () -> lower1)); + cases.add(new TypedDataSupplier("<" + lower1 + " long>", () -> lower1, DataTypes.LONG)); } return cases; } - private static List>> ulongCases(BigInteger min, BigInteger max) { - List>> cases = new ArrayList<>(); + private static List ulongCases(BigInteger min, BigInteger max) { + List cases = new ArrayList<>(); // Zero if (BigInteger.ZERO.compareTo(max) <= 0 && BigInteger.ZERO.compareTo(min) >= 0) { - cases.add(Map.entry("<0 unsigned long>", () -> BigInteger.ZERO)); + cases.add(new TypedDataSupplier("<0 unsigned long>", () -> BigInteger.ZERO, DataTypes.UNSIGNED_LONG)); } // small values, less than Long.MAX_VALUE @@ -495,13 +508,14 @@ private static List>> ulongCases(BigInteger m BigInteger upper1 = max.min(BigInteger.valueOf(Long.MAX_VALUE)); if (lower1.compareTo(upper1) < 0) { cases.add( - Map.entry( + new TypedDataSupplier( "", - () -> BigInteger.valueOf(ESTestCase.randomLongBetween(lower1.longValue(), upper1.longValue())) + () -> BigInteger.valueOf(ESTestCase.randomLongBetween(lower1.longValue(), upper1.longValue())), + DataTypes.UNSIGNED_LONG ) ); } else if (lower1.compareTo(upper1) == 0) { - cases.add(Map.entry("", () -> lower1)); + cases.add(new TypedDataSupplier("", () -> lower1, DataTypes.UNSIGNED_LONG)); } // Big values, greater than Long.MAX_VALUE @@ -509,51 +523,66 @@ private static List>> ulongCases(BigInteger m BigInteger upper2 = max.min(BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.valueOf(Integer.MAX_VALUE))); if (lower2.compareTo(upper2) < 0) { cases.add( - Map.entry( + new TypedDataSupplier( "", - () -> BigInteger.valueOf(ESTestCase.randomLongBetween(lower2.longValue(), upper2.longValue())) + () -> BigInteger.valueOf(ESTestCase.randomLongBetween(lower2.longValue(), upper2.longValue())), + DataTypes.UNSIGNED_LONG ) ); } else if (lower2.compareTo(upper2) == 0) { - cases.add(Map.entry("", () -> lower2)); + cases.add(new TypedDataSupplier("", () -> lower2, DataTypes.UNSIGNED_LONG)); } return cases; } - private static List>> doubleCases(double min, double max) { - List>> cases = new ArrayList<>(); + private static List doubleCases(double min, double max) { + List cases = new ArrayList<>(); // Zeros if (0d <= max && 0d >= min) { - cases.add(Map.entry("<0 double>", () -> 0.0d)); - cases.add(Map.entry("<-0 double>", () -> -0.0d)); + cases.add(new TypedDataSupplier("<0 double>", () -> 0.0d, DataTypes.DOUBLE)); + cases.add(new TypedDataSupplier("<-0 double>", () -> -0.0d, DataTypes.DOUBLE)); } // Positive small double double lower1 = Math.max(0d, min); double upper1 = Math.min(1d, max); if (lower1 < upper1) { - cases.add(Map.entry("", () -> ESTestCase.randomDoubleBetween(lower1, upper1, true))); + cases.add( + new TypedDataSupplier( + "", + () -> ESTestCase.randomDoubleBetween(lower1, upper1, true), + DataTypes.DOUBLE + ) + ); } else if (lower1 == upper1) { - cases.add(Map.entry("", () -> lower1)); + cases.add(new TypedDataSupplier("", () -> lower1, DataTypes.DOUBLE)); } // Negative small double double lower2 = Math.max(-1d, min); double upper2 = Math.min(0d, max); if (lower2 < upper2) { - cases.add(Map.entry("", () -> ESTestCase.randomDoubleBetween(lower2, upper2, true))); + cases.add( + new TypedDataSupplier( + "", + () -> ESTestCase.randomDoubleBetween(lower2, upper2, true), + DataTypes.DOUBLE + ) + ); } else if (lower2 == upper2) { - cases.add(Map.entry("", () -> lower2)); + cases.add(new TypedDataSupplier("", () -> lower2, DataTypes.DOUBLE)); } // Positive big double double lower3 = Math.max(1d, min); // start at 1 (inclusive) because the density of values between 0 and 1 is very high double upper3 = Math.min(Double.MAX_VALUE, max); if (lower3 < upper3) { - cases.add(Map.entry("", () -> ESTestCase.randomDoubleBetween(lower3, upper3, true))); + cases.add( + new TypedDataSupplier("", () -> ESTestCase.randomDoubleBetween(lower3, upper3, true), DataTypes.DOUBLE) + ); } else if (lower3 == upper3) { - cases.add(Map.entry("", () -> lower3)); + cases.add(new TypedDataSupplier("", () -> lower3, DataTypes.DOUBLE)); } // Negative big double @@ -561,48 +590,73 @@ private static List>> doubleCases(double min, double lower4 = Math.max(-Double.MAX_VALUE, min); double upper4 = Math.min(-1, max); // because again, the interval from -1 to 0 is very high density if (lower4 < upper4) { - cases.add(Map.entry("", () -> ESTestCase.randomDoubleBetween(lower4, upper4, true))); + cases.add( + new TypedDataSupplier("", () -> ESTestCase.randomDoubleBetween(lower4, upper4, true), DataTypes.DOUBLE) + ); } else if (lower4 == upper4) { - cases.add(Map.entry("", () -> lower4)); + cases.add(new TypedDataSupplier("", () -> lower4, DataTypes.DOUBLE)); } return cases; } - private static List>> booleanCases() { - return List.of(Map.entry("", () -> true), Map.entry("", () -> false)); + private static List booleanCases() { + return List.of( + new TypedDataSupplier("", () -> true, DataTypes.BOOLEAN), + new TypedDataSupplier("", () -> false, DataTypes.BOOLEAN) + ); } - private static List>> dateCases() { + private static List dateCases() { return List.of( - Map.entry("<1970-01-01T00:00:00Z>", () -> 0L), - Map.entry( + new TypedDataSupplier("<1970-01-01T00:00:00Z>", () -> 0L, DataTypes.DATETIME), + new TypedDataSupplier( "", - () -> ESTestCase.randomLongBetween(0, 10 * (long) 10e11) // 1970-01-01T00:00:00Z - 2286-11-20T17:46:40Z + () -> ESTestCase.randomLongBetween(0, 10 * (long) 10e11), // 1970-01-01T00:00:00Z - 2286-11-20T17:46:40Z + DataTypes.DATETIME ), - Map.entry( + new TypedDataSupplier( "", // 2286-11-20T17:46:40Z - +292278994-08-17T07:12:55.807Z - () -> ESTestCase.randomLongBetween(10 * (long) 10e11, Long.MAX_VALUE) + () -> ESTestCase.randomLongBetween(10 * (long) 10e11, Long.MAX_VALUE), + DataTypes.DATETIME ) ); } - private static List>> ipCases() { + private static List ipCases() { return List.of( - Map.entry("<127.0.0.1 ip>", () -> new BytesRef(InetAddressPoint.encode(InetAddresses.forString("127.0.0.1")))), - Map.entry("", () -> new BytesRef(InetAddressPoint.encode(ESTestCase.randomIp(true)))), - Map.entry("", () -> new BytesRef(InetAddressPoint.encode(ESTestCase.randomIp(false)))) + new TypedDataSupplier( + "<127.0.0.1 ip>", + () -> new BytesRef(InetAddressPoint.encode(InetAddresses.forString("127.0.0.1"))), + DataTypes.IP + ), + new TypedDataSupplier("", () -> new BytesRef(InetAddressPoint.encode(ESTestCase.randomIp(true))), DataTypes.IP), + new TypedDataSupplier("", () -> new BytesRef(InetAddressPoint.encode(ESTestCase.randomIp(false))), DataTypes.IP) ); } - private static List>> stringCases(String type) { - List>> result = new ArrayList<>(); - result.add(Map.entry("", () -> new BytesRef(""))); - result.add(Map.entry("", () -> new BytesRef(ESTestCase.randomAlphaOfLengthBetween(1, 30)))); - result.add(Map.entry("", () -> new BytesRef(ESTestCase.randomAlphaOfLengthBetween(300, 3000)))); - result.add(Map.entry("", () -> new BytesRef(ESTestCase.randomRealisticUnicodeOfLengthBetween(1, 30)))); + private static List stringCases(DataType type) { + List result = new ArrayList<>(); + result.add(new TypedDataSupplier("", () -> new BytesRef(""), type)); result.add( - Map.entry("", () -> new BytesRef(ESTestCase.randomRealisticUnicodeOfLengthBetween(300, 3000))) + new TypedDataSupplier("", () -> new BytesRef(ESTestCase.randomAlphaOfLengthBetween(1, 30)), type) + ); + result.add( + new TypedDataSupplier("", () -> new BytesRef(ESTestCase.randomAlphaOfLengthBetween(300, 3000)), type) + ); + result.add( + new TypedDataSupplier( + "", + () -> new BytesRef(ESTestCase.randomRealisticUnicodeOfLengthBetween(1, 30)), + type + ) + ); + result.add( + new TypedDataSupplier( + "", + () -> new BytesRef(ESTestCase.randomRealisticUnicodeOfLengthBetween(300, 3000)), + type + ) ); return result; } @@ -610,61 +664,27 @@ private static List>> stringCases(String type /** * Supplier test case data for {@link Version} fields. */ - public static List>> versionCases(String prefix) { + public static List versionCases(String prefix) { return List.of( - Map.entry("<" + prefix + "version major>", () -> new Version(Integer.toString(ESTestCase.between(0, 100))).toBytesRef()), - Map.entry( + new TypedDataSupplier( + "<" + prefix + "version major>", + () -> new Version(Integer.toString(ESTestCase.between(0, 100))).toBytesRef(), + DataTypes.VERSION + ), + new TypedDataSupplier( "<" + prefix + "version major.minor>", - () -> new Version(ESTestCase.between(0, 100) + "." + ESTestCase.between(0, 100)).toBytesRef() + () -> new Version(ESTestCase.between(0, 100) + "." + ESTestCase.between(0, 100)).toBytesRef(), + DataTypes.VERSION ), - Map.entry( + new TypedDataSupplier( "<" + prefix + "version major.minor.patch>", () -> new Version(ESTestCase.between(0, 100) + "." + ESTestCase.between(0, 100) + "." + ESTestCase.between(0, 100)) - .toBytesRef() + .toBytesRef(), + DataTypes.VERSION ) ); } - private static final Map>>> RANDOM_VALUE_SUPPLIERS = Map.ofEntries( - Map.entry( - DataTypes.DOUBLE, - List.of( - Map.entry("<0 double>", () -> 0.0d), - Map.entry("", () -> ESTestCase.randomDouble()), - Map.entry("", () -> -ESTestCase.randomDouble()), - Map.entry("", () -> ESTestCase.randomDoubleBetween(0, Double.MAX_VALUE, false)), - Map.entry("", () -> ESTestCase.randomDoubleBetween(Double.MIN_VALUE, 0 - Double.MIN_NORMAL, true)) - ) - ), - Map.entry( - DataTypes.LONG, - List.of( - Map.entry("<0 long>", () -> 0L), - Map.entry("", () -> ESTestCase.randomLongBetween(1, Long.MAX_VALUE)), - Map.entry("", () -> ESTestCase.randomLongBetween(Long.MIN_VALUE, -1)) - ) - ), - Map.entry( - DataTypes.INTEGER, - List.of( - Map.entry("<0 int>", () -> 0), - Map.entry("", () -> ESTestCase.between(1, Integer.MAX_VALUE)), - Map.entry("", () -> ESTestCase.between(Integer.MIN_VALUE, -1)) - ) - ), - Map.entry( - DataTypes.UNSIGNED_LONG, - List.of( - Map.entry("<0 unsigned long>", () -> BigInteger.ZERO), - Map.entry("", () -> BigInteger.valueOf(ESTestCase.randomLongBetween(1, Integer.MAX_VALUE))), - Map.entry( - "", - () -> BigInteger.valueOf(Long.MAX_VALUE).add(BigInteger.valueOf(ESTestCase.randomLongBetween(1, Integer.MAX_VALUE))) - ) - ) - ) - ); - private static String castToDoubleEvaluator(String original, DataType current) { if (current == DataTypes.DOUBLE) { return original; @@ -786,6 +806,13 @@ public TestCase withWarning(String warning) { } } + /** + * Holds a supplier for a data value, along with the type of that value and a name for generating test case names. This mostly + * exists because we can't generate random values from the test parameter generation functions, and instead need to return + * suppliers which generate the random values at test execution time. + */ + public record TypedDataSupplier(String name, Supplier supplier, DataType type) {} + /** * Holds a data value and the intended parse type of that value * @param data - value to test against diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionTests.java index c84f244b08df8..fefa397f7c77f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/convert/ToVersionTests.java @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.List; -import java.util.Map; import java.util.function.Supplier; import static org.hamcrest.Matchers.equalTo; @@ -51,9 +50,9 @@ public static Iterable parameters() { ); // But strings that are shaped like versions do parse to valid versions for (DataType inputType : EsqlDataTypes.types().stream().filter(EsqlDataTypes::isString).toList()) { - for (Map.Entry> versionGen : TestCaseSupplier.versionCases(inputType.typeName() + " ")) { - suppliers.add(new TestCaseSupplier(versionGen.getKey(), List.of(inputType), () -> { - BytesRef encodedVersion = (BytesRef) versionGen.getValue().get(); + for (TestCaseSupplier.TypedDataSupplier versionGen : TestCaseSupplier.versionCases(inputType.typeName() + " ")) { + suppliers.add(new TestCaseSupplier(versionGen.name(), List.of(inputType), () -> { + BytesRef encodedVersion = (BytesRef) versionGen.supplier().get(); TestCaseSupplier.TypedData typed = new TestCaseSupplier.TypedData( new BytesRef(new Version(encodedVersion).toString()), inputType, diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AcosTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AcosTests.java index bf1eeacc9ed06..12bc9c48827f5 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AcosTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AcosTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::acos, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AsinTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AsinTests.java index 119683d2d94e1..7cba8e88940c6 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AsinTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AsinTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::asin, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/Atan2Tests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/Atan2Tests.java index 3f4de813679da..0a884a2311e86 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/Atan2Tests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/Atan2Tests.java @@ -25,7 +25,17 @@ public Atan2Tests(@Name("TestCase") Supplier testCase @ParametersFactory public static Iterable parameters() { - List suppliers = TestCaseSupplier.forBinaryCastingToDouble("Atan2Evaluator", "y", "x", Math::atan2); + List suppliers = TestCaseSupplier.forBinaryCastingToDouble( + "Atan2Evaluator", + "y", + "x", + Math::atan2, + Double.NEGATIVE_INFINITY, + Double.POSITIVE_INFINITY, + Double.NEGATIVE_INFINITY, + Double.POSITIVE_INFINITY, + List.of() + ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AtanTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AtanTests.java index eb448b9402eea..897d4b18c3092 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AtanTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/AtanTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::atan, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CosTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CosTests.java index a024fbdb1d76f..c7b4570dab34f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CosTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CosTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::cos, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CoshTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CoshTests.java index 6d5393469422b..2a1e81b60a02f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CoshTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/CoshTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::cosh, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinTests.java index e9d0599842f98..788b506694d5e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::sin, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinhTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinhTests.java index 0f53ea76e8462..aad1e35a09da4 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinhTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/SinhTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::sinh, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanTests.java index 7e2bd466ffc10..1d654873f828f 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::tan, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanhTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanhTests.java index 1f6d5bd6e3b9f..a50fbfa642dd6 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanhTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/expression/function/scalar/math/TanhTests.java @@ -30,7 +30,8 @@ public static Iterable parameters() { "val", Math::tanh, Double.NEGATIVE_INFINITY, - Double.POSITIVE_INFINITY + Double.POSITIVE_INFINITY, + List.of() ); return parameterSuppliersFromTypedData(errorsForCasesWithoutExamples(anyNullIsNull(true, suppliers))); } From 2072be90b28c5e0fd7dff1ac26cec0b448cb7b48 Mon Sep 17 00:00:00 2001 From: Benjamin Trent Date: Tue, 12 Sep 2023 10:05:29 -0400 Subject: [PATCH 16/16] Utilize optimized dot_product where possible when calculating vector magnitude (#99448) Lucene provides an optimized `dot_product` calculation for vectors. We should use that when calculating a vector's magnitude. --- .../mapper/vectors/DenseVectorFieldMapper.java | 11 +++-------- .../index/mapper/vectors/VectorEncoderDecoder.java | 8 ++------ .../script/field/vectors/DenseVector.java | 14 ++++---------- 3 files changed, 9 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java index 0e4f871fbb8ca..bd9b9df68aff2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/vectors/DenseVectorFieldMapper.java @@ -26,6 +26,7 @@ import org.apache.lucene.search.KnnFloatVectorQuery; import org.apache.lucene.search.Query; import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.VectorUtil; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.fielddata.FieldDataContext; @@ -859,10 +860,7 @@ public Query createKnnQuery(byte[] queryVector, int numCands, Query filter, Floa } if (similarity == VectorSimilarity.DOT_PRODUCT || similarity == VectorSimilarity.COSINE) { - float squaredMagnitude = 0.0f; - for (byte b : queryVector) { - squaredMagnitude += b * b; - } + float squaredMagnitude = VectorUtil.dotProduct(queryVector, queryVector); elementType.checkVectorMagnitude(similarity, elementType.errorByteElementsAppender(queryVector), squaredMagnitude); } Query knnQuery = new KnnByteVectorQuery(name(), queryVector, numCands, filter); @@ -891,10 +889,7 @@ public Query createKnnQuery(float[] queryVector, int numCands, Query filter, Flo elementType.checkVectorBounds(queryVector); if (similarity == VectorSimilarity.DOT_PRODUCT || similarity == VectorSimilarity.COSINE) { - float squaredMagnitude = 0.0f; - for (float e : queryVector) { - squaredMagnitude += e * e; - } + float squaredMagnitude = VectorUtil.dotProduct(queryVector, queryVector); elementType.checkVectorMagnitude(similarity, elementType.errorFloatElementsAppender(queryVector), squaredMagnitude); } Query knnQuery = switch (elementType) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorEncoderDecoder.java b/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorEncoderDecoder.java index 381c1767edff3..e3285c4dc8644 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorEncoderDecoder.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/vectors/VectorEncoderDecoder.java @@ -9,6 +9,7 @@ package org.elasticsearch.index.mapper.vectors; import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.VectorUtil; import org.elasticsearch.index.IndexVersion; import java.nio.ByteBuffer; @@ -46,12 +47,7 @@ public static float decodeMagnitude(IndexVersion indexVersion, BytesRef vectorBR * Calculates vector magnitude */ private static float calculateMagnitude(float[] decodedVector) { - double magnitude = 0.0f; - for (int i = 0; i < decodedVector.length; i++) { - magnitude += decodedVector[i] * decodedVector[i]; - } - magnitude = Math.sqrt(magnitude); - return (float) magnitude; + return (float) Math.sqrt(VectorUtil.dotProduct(decodedVector, decodedVector)); } public static float getMagnitude(IndexVersion indexVersion, BytesRef vectorBR, float[] decodedVector) { diff --git a/server/src/main/java/org/elasticsearch/script/field/vectors/DenseVector.java b/server/src/main/java/org/elasticsearch/script/field/vectors/DenseVector.java index 84649d9954b6a..79a4c3fa1b2ee 100644 --- a/server/src/main/java/org/elasticsearch/script/field/vectors/DenseVector.java +++ b/server/src/main/java/org/elasticsearch/script/field/vectors/DenseVector.java @@ -8,6 +8,8 @@ package org.elasticsearch.script.field.vectors; +import org.apache.lucene.util.VectorUtil; + import java.util.List; /** @@ -151,11 +153,7 @@ default double cosineSimilarity(Object queryVector) { int size(); static float getMagnitude(byte[] vector) { - int mag = 0; - for (int elem : vector) { - mag += elem * elem; - } - return (float) Math.sqrt(mag); + return (float) Math.sqrt(VectorUtil.dotProduct(vector, vector)); } static float getMagnitude(byte[] vector, int dims) { @@ -170,11 +168,7 @@ static float getMagnitude(byte[] vector, int dims) { } static float getMagnitude(float[] vector) { - double mag = 0.0f; - for (float elem : vector) { - mag += elem * elem; - } - return (float) Math.sqrt(mag); + return (float) Math.sqrt(VectorUtil.dotProduct(vector, vector)); } static float getMagnitude(List vector) {