Skip to content

Conversation

@ibessonov
Copy link
Contributor

Signed-off-by: ibessonov <bessonov.ip@gmail.com>
Signed-off-by: ibessonov <bessonov.ip@gmail.com>
Signed-off-by: ibessonov <bessonov.ip@gmail.com>
Signed-off-by: ibessonov <bessonov.ip@gmail.com>
Copy link
Contributor

@SammyVimes SammyVimes left a comment

Choose a reason for hiding this comment

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

looks good to me

@Immutable
@Range(min = 1, max = Integer.MAX_VALUE)
@Value(hasDefault = true)
public int intId = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why such a name? seems to be more correct just id.
Why such a range?
Can we use UUID as an identifier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UUID is already a main "globally unique" identifier. This one - I'm not sure about the name, honestly. Field named "id" already exists.

public class TablesConfigurationSchema {
/** Global integer id counter. */
@Value(hasDefault = true)
public int globalIdCounter = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the global table identifier? why is it not clearly named? what happens if it overflows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'l expand the comment

* @param grpName Group name.
* @param grpId Group ID.
* @param tableName Table name.
* @param groupId Integer table id.
Copy link
Contributor

Choose a reason for hiding this comment

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

tableId ?

Signed-off-by: ibessonov <bessonov.ip@gmail.com>
Signed-off-by: ibessonov <bessonov.ip@gmail.com>
@ibessonov ibessonov merged commit d11a5fc into apache:main Jul 25, 2022
@ibessonov ibessonov deleted the ignite-16665 branch July 25, 2022 10:24
isapego pushed a commit to isapego/ignite-3 that referenced this pull request Dec 26, 2024
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.

3 participants