ziqiangliang commented on PR #7392: URL: https://github.com/apache/gravitino/pull/7392#issuecomment-3073725697
> 1. I think it makes sense to delay the execution of multiple operations and eventually close the sql session, which can reduce people's mistakes during development. > 2. The necessity of methods like `callWithoutCommit` / `opWithoutCommit`. My question is that the `Operation` class has been defined, and it seems that the usage is to combine multiple `Operations` and finally use them in `doMultipleWithCommit`. Is it redundant to provide a separate method to build `Operation`? > 3. If it is determined that the new design method is sufficient to solve the problem, should the previous method be set to `deprecated` and prompt people to use the new method first? > 4. I see that there are currently multiple places where sessions are still opened and not closed(such as some methods in `TagMetaService`), and the current PR has not been completely fixed. Can all repairs be completed in the current PR first? @ziqiangliang Can you check it again? > 1. I think it makes sense to delay the execution of multiple operations and eventually close the sql session, which can reduce people's mistakes during development. > 2. The necessity of methods like `callWithoutCommit` / `opWithoutCommit`. My question is that the `Operation` class has been defined, and it seems that the usage is to combine multiple `Operations` and finally use them in `doMultipleWithCommit`. Is it redundant to provide a separate method to build `Operation`? > 3. If it is determined that the new design method is sufficient to solve the problem, should the previous method be set to `deprecated` and prompt people to use the new method first? > 4. I see that there are currently multiple places where sessions are still opened and not closed(such as some methods in `TagMetaService`), and the current PR has not been completely fixed. Can all repairs be completed in the current PR first? @ziqiangliang Can you check it again? Regarding point 2, it might seem redundant at first, but methods like callWithoutCommit and opWithoutCommit exist to explicitly pass the mapper.class. This allows the operation’s function within Operation to directly receive a strongly-typed mapper instance (e.g., UserMapper), which improves readability and type safety by avoiding manual casting or lookups. Without these methods, users would need to manually handle mapper retrieval and casting inside their operations, increasing the chance of errors. While we could consider unifying these approaches in the future, for now, this pattern helps reduce mistakes and keeps the code clean and clear. Regarding point 3, we cannot directly add the @Deprecated annotation because it causes compilation errors. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
