FRiCKLE / ngx_postgres

upstream module that allows nginx to communicate directly with PostgreSQL database.
http://labs.frickle.com/nginx_ngx_postgres/
BSD 2-Clause "Simplified" License
545 stars 122 forks source link

postgres_pass Inheritance in nested locations #28

Open msva opened 10 years ago

msva commented 10 years ago

Hello there! I think, it would be great to make default behaviour to inherit postgres_pass setting from top-level location to it's nested locations.

For example, I assume that to work fine, instead of throwing 404:

location /db {
  postgres_pass   db;
  internal;
  location /db/test {
    postgres_query "SELECT id,name from users";
  }
}

Thanks a lot!

agentzh commented 10 years ago

@msva Will you provide a patch?

msva commented 10 years ago

I'm afraid, no 😞 At least, not now...

I'm almost not familiar with NginX API (not more than it needs for maintain a package and port some patches from old versions).

I tried to fix it myself in the way like it already done in rds_json module (where rds_json directive inherits to the nested locations just fine):

     { ngx_string("postgres_pass"),
       NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF|NGX_CONF_TAKE1,
       ngx_postgres_conf_pass,
       NGX_HTTP_LOC_CONF_OFFSET,
       offsetof(ngx_postgres_loc_conf_t, upstream),
       NULL },

but I still can't get it to work 😕

I also noticed, that postgress_output inherits fine too, but I can't realize the thing (except it provides a filter, just like rds_json) that makes them (with rds_json directive) works, while do not work for postgress_pass.

Maybe you can advice me, if I missed something else? 😇 Thanks! 😊

msva commented 8 years ago

By the way, I found strange thing. ngx_postgres_module.c contains following in ngx_postgres_merge_loc_conf function:

 358     if ((conf->upstream.upstream == NULL) && (conf->upstream_cv == NULL)) {
 359         conf->upstream.upstream = prev->upstream.upstream;
 360         conf->upstream_cv = prev->upstream_cv;
 361     }

And as I looked in ngx_postgres_conf_pass, it (postgresql_pass) sets either conf->upstream.upstream or conf->upstream_cv based of value type (is it complex or not), so if one of them set in the location, another should be NULL. And if neither set in the location, then code quoted above should (?) merge the ones that set on previous nesting level location (and so on for the top level). But somewhy it does not do that.

// by the way, it'd be nice to also allow _pass on server{} level.