A downside of this would be that we can't trim out the multi-host portion anymore?

A downside of this would be that we can't trim out the multi-host portion anymore?

I'm pretty sure nothing changed in that regard since you already can specify multiple hosts (and as a result, get a multi-host data source), it's just now you'll be able to control the way that data source will work.

Yeah, there is 0 difference in binary size with the main, so we already drag multiple hosts.

@vonzshik am trying to remember the exact context behind us doing this. I looked at #3495, and the EF story around multihost could indeed be improved, but I'm not sure exactly how allowing TargetSessionAttribute in the connection string helps... As I wrote in #3495, while it would make it possible to use NpgsqlDataSourceSource.Build() while passing TargetSessionAttribute, you'd end up with a single data source that has a single TargetSessionAttribute, and have no way to use the same physical connections with a different TargetSessionAttribute (e.g. a classical read-only and read-write setup).

EF users can still create their own multihost data source and get read-only/read-write wrappers (just like non-EF users), and then pass those to EF - it's not the prettiest, but it works. Otherwise someone who doesn't care about TargetSessionAttributes at all can also use multihost for failover/load balancing - they don't need to specify TargetSessionAttributes at all.

So what would be the user scenario where this is needed?

A downside of this would be that we can't trim out the multi-host portion anymore?

I'm pretty sure nothing changed in that regard since you already can specify multiple hosts (and as a result, get a multi-host data source), it's just now you'll be able to control the way that data source will work.

I think @NinoFloris was saying that if we do this, then we wouldn't ever be able to trim out multihost; I understand that we don't currently trim it out either (as you posted later), but in theory we could. On the other hand, IIRC we already accept multiple hosts in the connection string anyway, and that already triggers multihost, so also adding TargetSessionAttributes doesn't seem to make the current situation any worse. For example, we could say that the slim data source builder simply doesn't support multi-host at all except if you call BuildMultihost() (if you just call Build() we throw if multiple hosts and/or TargetSessionAttributes are specified).

EF users can still create their own multihost data source and get read-only/read-write wrappers (just like non-EF users), and then pass those to EF - it's not the prettiest, but it works. Otherwise someone who doesn't care about TargetSessionAttributes at all can also use multihost for failover/load balancing - they don't need to specify TargetSessionAttributes at all.

IMO for most EF consumers they shouldn't even make wrappers in the first place. Say, you have the standard primary-standby failover in place. So, to make it work with Npgsql ideally all you have to do is specify both hosts and then set TargetSessionAttributes to primary (because the default value is any). But currently you can't do that, you have to make a wrapper, and the worst thing, you can't change the behavior of the app from outside. Like, what if I have a read-only app, so by default I want to only read from a replica. But in some installations I might want to allow to read from primary, but then that would require changes in the code.

Thanks, makes sense (it has been a while since I've looked at all this).