> Instead, the approach on the PR seems to sort of "backdoor" in an API change without an ML thread by first making the persistence-related changes that optimize that API
I'm still not sure what that means. I asked in the PR what API you're suggesting and you never answered. Do you mean a new REST API? Or a PolarisMetaStoreManager change? I have no new REST endpoints in mind at this point and I don't think a REST endpoint is necessary to be able to take advantage of this information. > You gave the example of the task interface, but there have been several proposals to rework the task system exactly because it's not very well-engineered. Whether the task execution framework is well-engineered is irrelevant. It is a fact that parts of the service operate based on data stored in persistence that isn't shared over an externally facing API. This doesn't seem like a controversial topic to me. > In light of this its current state #2735 seems to be a sort of re-imagining of #433, but without any of the tangible benefits or measurements around perf. I think everyone agreed over the mailing list that the intended benefits of #433 are desirable, even if the implementation itself lacked rollback safety. #2735 is, in fact, a prerequisite to an alternative approach that we discussed several months ago, as is #2508. In fact, I'd really like to experiment with real numbers on a real deployment of Polaris to have a safe, efficient, performant implementation of TableMetadata in persistence, but that isn't ready yet. I'm not a fan of big-bang releases or giant PRs and I think small, easily digestible, easily rollback-able changes are safer and make for a better product. And given that both #2735 and #2508 offer benefits that can be taken advantage of even without the full implementation of #433, I don't see a reason to bundle all of these changes together. So I ask again, as I asked on the PR, what is the technical justification for the veto? If there's a specific concern, like rollback safety or memory usage, I'd be happy to hear it and make the appropriate changes. If the concern is simply that this doesn't fully complete the implementation of #433, I don't think that qualifies as a valid justification. Mike On Mon, Oct 13, 2025 at 11:28 AM Eric Maynard <[email protected]> wrote: > Hi Michael, > > I see the request on that PR as quite specific: > > > Wouldn't it make sense to design the API first before making persistence > changes? > > Instead, the approach on the PR seems to sort of "backdoor" in an API > change without an ML thread by first making the persistence-related changes > that optimize that API. Indeed, when the question was posed about what the > tangible benefit of the PR might be, you replied: > > > As an example, JB recently brought up working on the UI - it would be > nice if the UI could print some information about the table without loading > the entire metadata > > Presumably the UI would do this through some API, no? Or else how would > #2735 facilitate the UI doing this? However, you also commented: > > > What API? > > Not every field stored in the persistence layer serves a public-facing > API > > In light of this its current state #2735 seems to be a sort of re-imagining > of #433, but without any of the tangible benefits or measurements around > perf. As it stands, I don't see how the project benefits from merging #2735 > and tried to probe this with my questions on the PR. This question seems to > remain unaddressed. > > You did comment that the PR being blocked implied a question about whether > it was "unsafe or incorrect", which is confusing to me -- one could propose > adding to Polaris some class that correctly and safely implements image > rotation, but without a viable use-case for that class I would hope we > wouldn't merge that code. > > As to the other recent comment that it could be useful to add things to > persistence without any API exposing them, I'm not so sure about that. You > gave the example of the task interface, but there have been several > proposals to rework the task system exactly because it's not very > well-engineered. My understanding has historically been that the metastore > exists in service of the catalog service, which is exposed over a REST API. > Happy to discuss this more. > > In short, if there's no benefit to merging #2735, I don't see why we would > rush into merging it. If our intent is to add some API that this PR might > optimize, why not design, discuss, and implement this API first before > going ahead with a premature optimization? > > --EM > > > > On Mon, Oct 13, 2025 at 10:55 AM Michael Collado <[email protected]> > wrote: > > > Hey folks > > > > I got approval on https://github.com/apache/polaris/pull/2735 ten days > > ago, > > but a request for changes last Monday without any specific request or > > objection. I've asked a couple of times what the objection is, but > haven't > > heard a response. As far as I can see, there's no technical reason to > block > > this change. What's the community stance on this PR? At least three > people > > (aside from myself) have suggested that the feature is useful. Does > anyone > > feel that there is a technical reason for blocking this PR from merging? > If > > there is a concrete objection, I'm happy to hear it, but if people feel > > this is generally useful, I'd like to move forward with merging. > > > > Mke > > >
