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

Simplify Input Table Interface. #4923

Merged

Conversation

cpwright
Copy link
Contributor

@cpwright cpwright commented Dec 7, 2023

Removes the InputTableEnumGetter and InputTableRowSetter interfaces that MutableInputTable extended, but are not used by web.

There have been the following class renames:
io.deephaven.engine.util.config.MutableInputTable -> io.deephaven.engine.util.input.InputTableHandler
KeyedArrayBackedMutableTable -> io.deephaven.engine.table.impl.util.KeyedArrayBackedInputTable
AppendOnlyArrayBackedMutableTable -> io.deephaven.engine.table.impl.util.AppendOnlyArrayBackedInputTable

Additionally, the MutableInputTable interface, KeyedArrayBackedMutableTable, and AppendOnlyArrayBackedMutableTable no longer have parameters for enum values. The "allowEdits" parameter has been removed from the interface and all implementations. The delete method that takes a rowSet has been removed.

@cpwright cpwright requested a review from rcaudy December 7, 2023 17:50
@rcaudy rcaudy added query engine core Core development tasks DocumentationNeeded ReleaseNotesNeeded Release notes are needed labels Dec 7, 2023
@rcaudy rcaudy added this to the November 2023 milestone Dec 7, 2023
NullValueColumnSource.createColumnSourceMap(definition));
final AppendOnlyArrayBackedMutableTable result = new AppendOnlyArrayBackedMutableTable(
initialTable.getDefinition(), new ProcessPendingUpdater());
result.setAttribute(Table.ADD_ONLY_TABLE_ATTRIBUTE, Boolean.TRUE);
Copy link
Member

Choose a reason for hiding this comment

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

We should be setting append-only, too.

initialTable.getDefinition(), new ProcessPendingUpdater());
result.setAttribute(Table.ADD_ONLY_TABLE_ATTRIBUTE, Boolean.TRUE);
result.setFlat();
processInitial(initialTable, result);
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to create or process an empty initial table? Also, could one of the make overloads delegate to the other?

* <p>
* Implementations of this interface will make their own guarantees about how atomically changes will be applied and
* what operations they support.
*/
public interface MutableInputTable {
public interface InputTable {
Copy link
Member

Choose a reason for hiding this comment

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

InputTableHandler?

Copy link
Contributor

@jmao-denver jmao-denver left a comment

Choose a reason for hiding this comment

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

Python changes LGTM

@cpwright cpwright merged commit 1607606 into deephaven:main Dec 8, 2023
12 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
@deephaven-internal
Copy link
Contributor

Labels indicate documentation is required. Issues for documentation have been opened:

How-to: https://github.com/deephaven/deephaven.io/issues/3528
Reference: https://github.com/deephaven/deephaven.io/issues/3529

@rcaudy rcaudy added the breaking label Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants