add: config to emit warning for legacy target names

[steve-chavez]

Reverts #4104 and eases transition towards the breaking change.

When the request with the deprecated syntax comes we log a warning, like so:

$ PGRST_LOG_LEVEL=info postgrest-with-pg-14 -f test/spec/fixtures/load.sql postgrest-run

$ curl 'localhost:3000/projects?id=eq.1&select=id,name,the_tasks:tasks(id,name),clientes:clients(*)&tasks.order=name.asc&clients.id=eq.1'

08/May/2026:21:10:24 -0500: WARNING: Embedded resource was referenced by relation name even though it has an alias. This is deprecated and will stop working in a future release.
08/May/2026:21:10:24 -0500: Please update the filters that use `tasks` to `the_tasks`, `clients` to `clientes` in `GET /projects?id=eq.1&select=id,name,the_tasks:tasks(id,name),clientes:client
s(*)&tasks.order=name.asc&clients.id=eq.1`
127.0.0.1 - postgrest_test_anonymous [08/May/2026:21:10:24 -0500] "GET /projects?id=eq.1&select=id,name,the_tasks:tasks(id,name),clientes:clients(*)&tasks.order=name.asc&clients.id=eq.1 HTTP/1
.1" 200 146 "" "curl/7.81.0"

Note that:

• The request is repeated on the WARNING message. This is because the error logs go to stderr and access/apache logs to stdout, we can't ensure they appear at the same time.
• Includes all the alias that need to be corrected "tasks to the_tasks, clients to clientes"

TODO

• Add a config that allows user to opt in to this breaking change and removes the warning. Let's have a single config for all cases that can come up in the future. Like server-legacy-features=target-names.
• Tests. fix: unexpected results when embedding the same table twice #4104 didn't include a test that confirms old target names didn't work.
• Loadtests. Add a test in test/load/targets.http that uses the alias to see the impact of this new logic. This should be done in another PR.
• The warning only happens for log-level=warn, this requires Switch to log-level=warn as default #4904 to be effective. Not planned anymore, see also: add: config to emit warning for legacy target names #4905 (comment)

[laurenceisla]

Includes all the alias that need to be corrected "tasks to the_tasks, clients to clientes"

While working on building this warning, I noticed that we might avoid the breaking change if we look for the filter twice in the list of embeddings:

1. We check if the relation alias is present in the embeddings list and use the filter on that embedding if found
2. If not, then we make the search in the list again, but we look for relation names now

All tests pass when I try this, including the one that revealed the breaking change, but there could be outliers that may still show a breaking change so I'd have to look for them. I think it's worth the try, but before doing that @steve-chavez do you see it as a viable alternative against this whole breaking change warning stuff? (this would still be useful for future breaking changes though)

[steve-chavez]

@laurenceisla I'd say go ahead if you think it's doable without a breaking change (not sure if possible). But we do need to add loadtests for these alias cases to see if the double searching you propose won't cause a noticeable regression.

[wolfgangwalther]

I think we should do the breaking change at some point anyway. The previous behavior was just wrong - once you rename it via an alias, that's the name it should go by. If you want another release with a warning etc. I'd be OK with that. But I'd also be OK without that added complexity and the status quo (the breaking fix as it is currently done on main).

[laurenceisla]

(not sure if possible)
The previous behavior was just wrong

Right, after pondering it for some time, I believe the breaking change is inevitable. Will continue with the TODO list for this PR.

[steve-chavez]

Now that #4904 is closed. Can we still emit the WARNING: logs I mentioend above? I'd think so given it's critical for the administrator to somehow communicate this to clients.

@wolfgangwalther WDYT?

[steve-chavez]

@laurenceisla How about merging the first revert in another PR? I think we agree that it's not right to make the breaking change this way. Otherwise we risk this going into a new release somehow.

[wolfgangwalther]

Can we still emit the WARNING: logs I mentioend above? I'd think so given it's critical for the administrator to somehow communicate this to clients.

@wolfgangwalther WDYT?

As I wrote in an earlier comment, I am not opposed to that. I don't feel like it's necessary, but if you do, I'm ok with it.

I think we should not revert it - because we will need to do it anyway, for correctness. So we better deal with it in a way that makes you feel good about it.

[laurenceisla]

This should be ready for review now.

Loadtests. Add a test in test/load/targets.http that uses the alias to see the impact of this new logic. This should be done in another PR.

Do you mean to test how it's currently working on main and to check the impact of this PR?

[wolfgangwalther]

Loadtests. Add a test in test/load/targets.http that uses the alias to see the impact of this new logic. This should be done in another PR.

Do you mean to test how it's currently working on main and to check the impact of this PR?

Just a note: The tests for the loadtest are always taken from the head branch. I.e., if you add a test in this PR, you will run it on all 3 branches under test, so you don't need a separate PR to get results from the new test.

[steve-chavez]

Do you mean to test how it's currently working on main and to check the impact of this PR?

@laurenceisla I meant adding a new request like /x?select=alias:table(*)&alias.id=eq.1 (2 or 3 aliased filters would be good) in test/load/targets.http, to check if the additional logic we add to make the fix conditional has an impact on performance.

[laurenceisla]

I added a loadtest and here are the results: https://github.com/PostgREST/postgrest/actions/runs/26861796165. It cannot compare to 200 HTTP responses in main because that request fails in main. However, compared to 14.12 it appears to be a slight increase but I think it's negligible, right? (a couple to a few μs).

[steve-chavez]

However, compared to 14.12 it appears to be a slight increase but I think it's negligible, right? (a couple to a few μs).

Right, I think there's no other way anyway, we'll also remove this logic on the next major. Then the slight perf drop will go away.

@laurenceisla Could you rebase this PR so the revert and 533dec4 are no longer included? We don't need them right?

[laurenceisla]

@laurenceisla Could you rebase this PR so the revert and 533dec4 are no longer included? We don't need them right?

Correct. I found it difficult to separate the "new config" commit from the "emit warning" commit without the whole "revert"->"readd" thing, so I squashed them into a single one. In fact maybe all of the commits in this PR should be squashed when merged?

[steve-chavez]

@laurenceisla Yes, can you squash? 🙏 We've disabled squash/merge from github UI.

[laurenceisla]

Addressed what was mentioned here #4905 (comment)

However I'm still leaving it on draft. Awaiting the follow up of #4905 (comment)

Edit: Should be ready for review again

[steve-chavez]

Hm, this is confusing as a feature here. I think it should be enough to list it on the fixes.

If we want to make it more obvious, maybe we should add a new "Migrating to v16" how-to?

[laurenceisla]

I think it should be enough to list it on the fixes.

Right, I'll remove it.

maybe we should add a new "Migrating to v16" how-to?

Should we include it in this PR? Or as a separate one when we decide to release the new version. I assume we'd like to mention other things apart from this config.

[steve-chavez]

Yeah, should be done in a separate PR.

[steve-chavez]

@laurenceisla Can't approve through github UI because I'm the PR author 😞.

But I do approve ✅, this can be merged.

[taimoorzaeem]

We would need to remember to remove this for v18. How about we create a dedicated issue to track all deprecated features/changes that need to removed? We can add that to the v18 milestone too.

Also, I think the sooner we remove these deprecated feats/changes the better it would be for maintainability.

[steve-chavez]

Note that we don't have to do this for next major, we documented:

for at least one MAJOR
https://docs.postgrest.org/en/v16/index.html#deprecation-policy

This is a very difficult migration, I'd say we need at least 2 majors or 1 year.

[wolfgangwalther]

Wasn't there the intermediate step of changing the default value for the new config option that we wanted to do for v18? v20 should then remove it entirely.

[steve-chavez]

Noted a defficiency here, the log is not debounced, so if several of these deprecated requests come they could degrade the service.

#5203 has a similar problem.

[wolfgangwalther]

the log is not debounced

See #5203 (comment) for my opinion on debounced, non-deterministic log output.