yuqi1129 commented on PR #12350: URL: https://github.com/apache/gravitino/pull/12350#issuecomment-5191477739
> > @roryqi > > Thanks for the suggestion. I understand it as extracting a common OCC SQL template for update/fence/soft-delete operations, with each provider supplying the table name, ID column, and MyBatis parameter names. After reviewing these statements, I think a generic SQL template would have limited benefit here. Besides the table and ID column, the MyBatis parameter paths, batch collection names, entity-specific SET clauses, and database-specific deleted_at expressions also differ. Passing these as string parameters would hide the final SQL, reduce readability, and move some errors to runtime. I would prefer to keep the entity-specific SQL explicit. > > I agree that exact duplication should be removed. A narrower approach would be to reuse DatabaseTimeSQL for database-time expressions and let PostgreSQL providers inherit SQL that is identical to the base provider, keeping overrides only where the SQL dialect actually differs. If more entities later converge on an identical OCC SQL shape, we can revisit a narrowly scoped helper. Would this narrower cleanup address your concern, or do you have a specific interface/template design in mind? > > I prefer think further. We can give more constraints about table create SQL standard. Some common column must has the fixed name. Some services must have some specific interfaces. > > For example, we have delete_at column in every table, we have a delete interface. Every storage service should implement it. > > I think this is possible. We would better have a framework to handle this issue. We should think more about how to iterate our storage framework. Now, we are using AI to generate the code. It seems not bring much burden. But it is still hard for people to review code and maintain our code base. @roryqi Thanks for the suggestion. I understand it as extracting a common OCC SQL template for update/fence/soft-delete operations, with each provider supplying the table name, ID column, and MyBatis parameter names. After reviewing these statements, I think a generic SQL template would have limited benefit here. Besides the table and ID column, the MyBatis parameter paths, batch collection names, entity-specific SET clauses, and database-specific deleted_at expressions also differ. Passing these as string parameters would hide the final SQL, reduce readability, and move some errors to runtime. I would prefer to keep the entity-specific SQL explicit. I agree that exact duplication should be removed. A narrower approach would be to reuse DatabaseTimeSQL for database-time expressions and let PostgreSQL providers inherit SQL that is identical to the base provider, keeping overrides only where the SQL dialect actually differs. If more entities later converge on an identical OCC SQL shape, we can revisit a narrowly scoped helper. Would this narrower cleanup address your concern, or do you have a specific interface/template design in mind? I prefer think further. We can give more constraints about table create SQL standard. Some common column must has the fixed name. Some services must have some specific interfaces. For example, we have delete_at column in every table, we have a delete interface. Every storage service should implement it. I think this is possible. We would better have a framework to handle this issue. We should think more about how to iterate our storage framework. Now, we are using AI to generate the code. It seems not bring much burden. But it is still hard for people to review code and maintain our code base. I suggest we use another PR to discuss the problem and leave your ideas in as much detail as possible. AI hinted to me that - It's not so proper to do it in this PR - The final return is unclear. If you insist on this point, please just create an issue that is targeted at this problem. I think we can do a pure refactor without this PR. -- 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]
