singhpk234 commented on PR #1802: URL: https://github.com/apache/polaris/pull/1802#issuecomment-2949918614
Thank you so much every one for the quick review, I tried addressing most of the feedback here : Re: Static SQL - I opened a pr https://github.com/apache/polaris/pull/1824 have moved everything to the SQL Constants to make this entirely static Re: Realm-id as part of model - I addressed this in https://github.com/apache/polaris/pull/1824 as well now the model will contain the realm-id as the part of model itself, i didn't intially put this as PolarisBaseEntity has no way of realm-id, but more i think of it now and as called out in the PR feedbacks that the right way to go ! Re: DatabaseType - I agree this is leaking rightnow in the model, let me think of a more cleaner way to abstract it, I like @dimas-b suggestion to make the converter at from the type level, let me take stab in this refactoring. I hope this addresses major concern around this PR and i sincerly thank the whole community to jump in and help here ! -- 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: issues-unsubscr...@polaris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
