Skip to content
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

Add Transform reset logic #2

Merged

Conversation

gwbrown
Copy link

@gwbrown gwbrown commented Feb 25, 2021

This is a draft of reset logic for Transforms. It's not on the main Elasticsearch repo as it depends on infrastructure which has not yet been merged (elastic#69469), but I would like to get eyes on it and this is the easiest way to do so.

Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, after that indexes must be deleted, but we can do this in a follow-up

@gwbrown
Copy link
Author

gwbrown commented Mar 2, 2021

Thanks for the review @hendrikmuhs! This will be merged as part of elastic#69469.

Regarding deleting the indices, after stopping the transforms, we call into the default implementation of cleanUpFeature, which deletes all the system indices and associated indices registered to that plugin - unless there's specific index cleanup other than that, plugins should rely on that default logic to delete the indices in question.

@gwbrown gwbrown merged commit a61cf38 into williamrandolph:si/reset-features-api Mar 3, 2021
williamrandolph pushed a commit that referenced this pull request Mar 15, 2021
…astic#69765)

Previously we did not resolve the attributes recursively which meant that if a field or expression was re-aliased multiple times (through multiple levels of subqueries), the aliases were only resolved one level down. This led to failed query translation because `ReferenceAttribute`s were pointing to non-existing attributes during query translation.

For example the query

```sql
SELECT i AS j FROM ( SELECT int AS i FROM test) ORDER BY j
```

failed during translation because the `OrderBy` resolved the `j` ReferenceAttribute to another `i` ReferenceAttribute that was later removed by an Optimization:

```
OrderBy[[Order[j{r}#4,ASC,LAST]]]                                             ! OrderBy[[Order[i{r}#2,ASC,LAST]]]
\_Project[[j]]                                                                = \_Project[[j]]
  \_Project[[i]]                                                              !   \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..]
    \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..] ! 
```

By resolving the `Attributes` recursively both `j{r}` and `i{r}` will resolve to `test.int{f}` above:

```
OrderBy[[Order[test.int{f}elastic#22,ASC,LAST]]]                                     = OrderBy[[Order[test.int{f}elastic#22,ASC,LAST]]]
\_Project[[j]]                                                                = \_Project[[j]]
  \_Project[[i]]                                                              !   \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..]
    \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..] ! 
 ```

The scope of recursive resolution depends on how the `AttributeMap` is constructed and populated.

Fixes elastic#67237
williamrandolph pushed a commit that referenced this pull request Mar 17, 2021
…astic#69765) (elastic#70322)

Previously we did not resolve the attributes recursively which meant that if a field or expression was re-aliased multiple times (through multiple levels of subqueries), the aliases were only resolved one level down. This led to failed query translation because `ReferenceAttribute`s were pointing to non-existing attributes during query translation.

For example the query

```sql
SELECT i AS j FROM ( SELECT int AS i FROM test) ORDER BY j
```

failed during translation because the `OrderBy` resolved the `j` ReferenceAttribute to another `i` ReferenceAttribute that was later removed by an Optimization:

```
OrderBy[[Order[j{r}#4,ASC,LAST]]]                                             ! OrderBy[[Order[i{r}#2,ASC,LAST]]]
\_Project[[j]]                                                                = \_Project[[j]]
  \_Project[[i]]                                                              !   \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..]
    \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..] ! 
```

By resolving the `Attributes` recursively both `j{r}` and `i{r}` will resolve to `test.int{f}` above:

```
OrderBy[[Order[test.int{f}elastic#22,ASC,LAST]]]                                     = OrderBy[[Order[test.int{f}elastic#22,ASC,LAST]]]
\_Project[[j]]                                                                = \_Project[[j]]
  \_Project[[i]]                                                              !   \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..]
    \_EsRelation[test][date{f}elastic#6, some{f}elastic#7, some.string{f}elastic#8, some.string..] ! 
 ```

The scope of recursive resolution depends on how the `AttributeMap` is constructed and populated.

Fixes elastic#67237
williamrandolph pushed a commit that referenced this pull request Oct 11, 2023
…c#100592)

* Don't print synthetic source in mapping for bwc tests

* Move comment.

* Don't print synthetic source in mapping for bwc tests #2

* Don't print synthetic source in mapping for bwc tests #2

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 034262c.

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 44e8156.

* Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)"

This reverts commit 9322ab9.

* Exclude synthetic source test from mixedClusterTests

* Update comment.
williamrandolph pushed a commit that referenced this pull request Oct 13, 2023
….10 (elastic#100805)

* Don't print synthetic source in mapping for bwc tests

* Move comment.

* Don't print synthetic source in mapping for bwc tests #2

* Don't print synthetic source in mapping for bwc tests #2

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 034262c.

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 44e8156.

* Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)"

This reverts commit 9322ab9.

* Exclude synthetic source test from mixedClusterTests

* Update comment.

* Mute all tsdb tests in mixedClusterTests

This is an interim step to stop sporadic test failures, while we try to
fix version skip for mixed cluster tests.

* Remove old exclusion

* Add aggregations too

* Mute tests for versions between 8.7-8.10

* Remove mute
williamrandolph pushed a commit that referenced this pull request Oct 13, 2023
* Don't print synthetic source in mapping for bwc tests

* Move comment.

* Don't print synthetic source in mapping for bwc tests #2

* Don't print synthetic source in mapping for bwc tests #2

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 034262c.

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 44e8156.

* Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)"

This reverts commit 9322ab9.

* Exclude synthetic source test from mixedClusterTests

* Update comment.

* Mute all tsdb tests in mixedClusterTests

This is an interim step to stop sporadic test failures, while we try to
fix version skip for mixed cluster tests.

* Remove old exclusion

* Add aggregations too

* Mute tests for versions between 8.7-8.10

* Remove mute

* Restore version skipping for position fields

* Restore version skip for synthetic source
williamrandolph pushed a commit that referenced this pull request Nov 1, 2023
….10 (elastic#100805) (elastic#100814)

* Don't print synthetic source in mapping for bwc tests

* Move comment.

* Don't print synthetic source in mapping for bwc tests #2

* Don't print synthetic source in mapping for bwc tests #2

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 034262c.

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 44e8156.

* Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)"

This reverts commit 9322ab9.

* Exclude synthetic source test from mixedClusterTests

* Update comment.

* Mute all tsdb tests in mixedClusterTests

This is an interim step to stop sporadic test failures, while we try to
fix version skip for mixed cluster tests.

* Remove old exclusion

* Add aggregations too

* Mute tests for versions between 8.7-8.10

* Remove mute
williamrandolph pushed a commit that referenced this pull request Nov 1, 2023
…c#100823)

* Don't print synthetic source in mapping for bwc tests

* Move comment.

* Don't print synthetic source in mapping for bwc tests #2

* Don't print synthetic source in mapping for bwc tests #2

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 034262c.

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 44e8156.

* Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)"

This reverts commit 9322ab9.

* Exclude synthetic source test from mixedClusterTests

* Update comment.

* Mute all tsdb tests in mixedClusterTests

This is an interim step to stop sporadic test failures, while we try to
fix version skip for mixed cluster tests.

* Remove old exclusion

* Add aggregations too

* Mute tests for versions between 8.7-8.10

* Remove mute

* Restore version skipping for position fields

* Restore version skip for synthetic source
williamrandolph pushed a commit that referenced this pull request Nov 1, 2023
…8.7 - 8.10 (elastic#100805) (elastic#100815)

* [TEST] Mute all tsdb tests in mixedClusterTests, for versions 8.7 - 8.10 (elastic#100805)

* Don't print synthetic source in mapping for bwc tests

* Move comment.

* Don't print synthetic source in mapping for bwc tests #2

* Don't print synthetic source in mapping for bwc tests #2

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 034262c.

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 44e8156.

* Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)"

This reverts commit 9322ab9.

* Exclude synthetic source test from mixedClusterTests

* Update comment.

* Mute all tsdb tests in mixedClusterTests

This is an interim step to stop sporadic test failures, while we try to
fix version skip for mixed cluster tests.

* Remove old exclusion

* Add aggregations too

* Mute tests for versions between 8.7-8.10

* Remove mute

* Fix skip for position fields
williamrandolph pushed a commit that referenced this pull request Nov 1, 2023
…rce (elastic#100827)

* [TEST] Mute all tsdb tests in mixedClusterTests, for versions 8.7 - 8.10 (elastic#100805)

* Don't print synthetic source in mapping for bwc tests

* Move comment.

* Don't print synthetic source in mapping for bwc tests #2

* Don't print synthetic source in mapping for bwc tests #2

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 034262c.

* Revert "Don't print synthetic source in mapping for bwc tests #2"

This reverts commit 44e8156.

* Revert "Don't print synthetic source in mapping for bwc tests (elastic#100572)"

This reverts commit 9322ab9.

* Exclude synthetic source test from mixedClusterTests

* Update comment.

* Mute all tsdb tests in mixedClusterTests

This is an interim step to stop sporadic test failures, while we try to
fix version skip for mixed cluster tests.

* Remove old exclusion

* Add aggregations too

* Mute tests for versions between 8.7-8.10

* Remove mute

* Fix skip for position fields

* Restore version skip for synthetic source
williamrandolph pushed a commit that referenced this pull request Mar 14, 2024
When we run the csv-spec tests for ESQL against a real http endpoint we
actually run them twice - once async and once sync. But the names of the
tests didn't reflect that - they just looked like they were accidentally
duplicated. This updates the format. So this:
```
test {string.Trim}
test {string.Trim #2}
```

becomes:
```
test {string.Trim ASYNC}
test {string.Trim SYNC}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants