You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by "Bob Dionne (JIRA)" <ji...@apache.org> on 2011/01/02 14:06:45 UTC

[jira] Created: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

list_to_existing_atom is too restrictive as used by couch_rep
-------------------------------------------------------------

                 Key: COUCHDB-1004
                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
             Project: CouchDB
          Issue Type: Bug
          Components: Replication
         Environment: erlang
            Reporter: Bob Dionne
            Priority: Minor


We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...

The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Created: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by Filipe David Manana <fd...@apache.org>.
On Sun, Jan 2, 2011 at 1:48 PM, Robert Dionne
<di...@dionne-associates.com> wrote:
> Klaus,
>
>   perhaps I just heard wrong or misinterpreted what was said in the chat room. It did seem unusual that calling list_to_atom("foo") twice would add more than one atom.

I was wrong about that. I had the idea that I saw or heard from
someone else once that two calls to list_to_atom("foobar") would
create 2 entries in the atom table. Perhaps this was true with older
erlang/otp versions, but certainly not with r14 at least:

Eshell V5.8.2  (abort with ^G)
1> memory(atom_used).
423042
2> list_to_atom("foobar").
foobar
3> memory(atom_used).
430855
4> list_to_atom("foobar").
foobar
5> memory(atom_used).
430855

Going directly to the source, confirms it as well:
https://github.com/erlang/otp/blob/dev/erts/emulator/beam/atom.c#L223


>
> On Jan 2, 2011, at 8:26 AM, Klaus Trainer wrote:
>
>> As far as I can remember, the motivation behind list_to_existing_atom
>> was not the fact that list_to_atom pollutes the atoms table during
>> normal operation. However, it won't prevent atom table pollution when
>> something goes wrong or somebody goes malicious (i.e., DoS attack).
>>
>> I've just looked it up for you, the exact description is here:
>> https://issues.apache.org/jira/browse/COUCHDB-829
>>
>>
>> - Klaus
>>
>>
>> On Sun, 2011-01-02 at 08:06 -0500, Bob Dionne (JIRA) wrote:
>>> list_to_existing_atom is too restrictive as used by couch_rep
>>> -------------------------------------------------------------
>>>
>>>                 Key: COUCHDB-1004
>>>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>>>             Project: CouchDB
>>>          Issue Type: Bug
>>>          Components: Replication
>>>         Environment: erlang
>>>            Reporter: Bob Dionne
>>>            Priority: Minor
>>>
>>>
>>> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
>>>
>>> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.
>>>
>>
>>
>
>



-- 
Filipe David Manana,
fdmanana@gmail.com, fdmanana@apache.org

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [jira] Created: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by Robert Dionne <di...@dionne-associates.com>.
Klaus,

   perhaps I just heard wrong or misinterpreted what was said in the chat room. It did seem unusual that calling list_to_atom("foo") twice would add more than one atom. So just reverting the call back in couch_rep:dbinfo should suffice to fix this as it's internal. Thanks,

Bob



On Jan 2, 2011, at 8:26 AM, Klaus Trainer wrote:

> As far as I can remember, the motivation behind list_to_existing_atom
> was not the fact that list_to_atom pollutes the atoms table during
> normal operation. However, it won't prevent atom table pollution when
> something goes wrong or somebody goes malicious (i.e., DoS attack).
> 
> I've just looked it up for you, the exact description is here:
> https://issues.apache.org/jira/browse/COUCHDB-829
> 
> 
> - Klaus
> 
> 
> On Sun, 2011-01-02 at 08:06 -0500, Bob Dionne (JIRA) wrote:
>> list_to_existing_atom is too restrictive as used by couch_rep
>> -------------------------------------------------------------
>> 
>>                 Key: COUCHDB-1004
>>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>>             Project: CouchDB
>>          Issue Type: Bug
>>          Components: Replication
>>         Environment: erlang
>>            Reporter: Bob Dionne
>>            Priority: Minor
>> 
>> 
>> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
>> 
>> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.
>> 
> 
> 


Re: [jira] Created: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by Klaus Trainer <kl...@web.de>.
As far as I can remember, the motivation behind list_to_existing_atom
was not the fact that list_to_atom pollutes the atoms table during
normal operation. However, it won't prevent atom table pollution when
something goes wrong or somebody goes malicious (i.e., DoS attack).

I've just looked it up for you, the exact description is here:
https://issues.apache.org/jira/browse/COUCHDB-829


- Klaus


On Sun, 2011-01-02 at 08:06 -0500, Bob Dionne (JIRA) wrote:
> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
> 
>                  Key: COUCHDB-1004
>                  URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>              Project: CouchDB
>           Issue Type: Bug
>           Components: Replication
>          Environment: erlang
>             Reporter: Bob Dionne
>             Priority: Minor
> 
> 
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> 
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.
> 



[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976561#action_12976561 ] 

Randall Leeds commented on COUCHDB-1004:
----------------------------------------

Adam, if I follow you right you're saying catch the badarg on list_to_existing_atom/1 and leave it as binray. Sounds like a good plan to me.

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978519#action_12978519 ] 

Adam Kocoloski commented on COUCHDB-1004:
-----------------------------------------

Yep, I know.  What I meant was that an alternative solution which left everything as atoms and used couch_util:to_existing_atom/1 would not need to do the double-get_value thing, because the lookup for the binary key would never succeed.

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Updated] (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Jan Lehnardt (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Jan Lehnardt updated COUCHDB-1004:
----------------------------------

    Fix Version/s: 1.2
                   1.1
                   1.0.3

Bump, should we get this into 1.0.3/1.1.0 now?

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>             Fix For: 1.0.3, 1.1, 1.2
>
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976556#action_12976556 ] 

Adam Kocoloski commented on COUCHDB-1004:
-----------------------------------------

A header is certainly doable from the BigCouch perspective.  But in general I don't like the fact that we've painted ourselves into an API corner where we cannot extend db.info() in the future without breaking replication.

I think to_existing_atom/1 is a fine replacement in this case.  If it's an unknown atom we have a guarantee that the replicator is not going to be looking for that particular field, so we might as well just leave it as a binary.

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Filipe Manana (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978517#action_12978517 ] 

Filipe Manana commented on COUCHDB-1004:
----------------------------------------

Adam, that code snippet is not used by the proposed solution :)

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Filipe Manana (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Filipe Manana updated COUCHDB-1004:
-----------------------------------

    Attachment: COUCHDB-1004.patch

Here's the proposed solution

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Resolved] (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Filipe Manana (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Filipe Manana resolved COUCHDB-1004.
------------------------------------

       Resolution: Fixed
    Fix Version/s:     (was: 1.2)

Applied Adam's suggestion (use couch_util:to_existing_atom/1) to 1.0.x and 1.1.x.
Trunk does not suffer from this issue.

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>             Fix For: 1.0.3, 1.1
>
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Filipe Manana (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978306#action_12978306 ] 

Filipe Manana commented on COUCHDB-1004:
----------------------------------------

I now remember I had the same issue with the new replicator code. I opted for using always binaries:

https://github.com/fdmanana/couchdb/blob/trunk_new_replicator/src/couchdb/couch_api_wrap.erl#L110

Like this, all the code that uses the output of that function always does something like:

couch_util:get_value(<<"key">>, List)

Using couch_util:to_existing_atom/1, means that further code doesn't know what type of key to use: an atom or a binary. The solution would be to do something like:

couch_util:get_value(mykey, List, couch_util:get_value(<<"mykey">>, List))

But that's a bit too much typing imho

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Adam Kocoloski updated COUCHDB-1004:
------------------------------------

    Skill Level: New Contributors Level (Easy)

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Randall Leeds (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976553#action_12976553 ] 

Randall Leeds commented on COUCHDB-1004:
----------------------------------------

I just thought of one alternative.
BigCouch could send an extra header with the dbinfo request. Couch would ignore this header while BigCouch would use it to decide whether to send the extra info.

Just a thought.

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Bob Dionne (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978343#action_12978343 ] 

Bob Dionne commented on COUCHDB-1004:
-------------------------------------

Thanks Filipe,

  this solves our bigcouch issue and I'd be +1 on having it included in 1.0.2,

Bob

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Filipe Manana (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976513#action_12976513 ] 

Filipe Manana commented on COUCHDB-1004:
----------------------------------------

I agree. In some cases it's not really an issue.
I would vote to replace our to_existing_atom/1 function in couch_util with something like:

%% @spec to_atom(term(), allow_new())
%% @type allow_new() = boolean()
to_atom(V, false) ->
    list_to_existing_atom(to_list(V));
to_atom(V, true) ->
    V1 = to_list(V),
    try
        list_to_existing_atom(V1)
    catch _:_ ->
        list_to_atom(V1)
    end.



> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] [Commented] (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Bob Dionne (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13020772#comment-13020772 ] 

Bob Dionne commented on COUCHDB-1004:
-------------------------------------

It would nice, and helpful downstream. I think Adam's proposed trivial change is the simplest, use couch_util:to_existing_atom/1

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>             Fix For: 1.0.3, 1.1, 1.2
>
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Filipe Manana (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978522#action_12978522 ] 

Filipe Manana commented on COUCHDB-1004:
----------------------------------------

I see what you mean. When it didn't work for me it was maybe because the get_value(atom, ...) was being done on a module other than the module which did the calls to list_to_existing_atom/1. Long time ago and a different code/context.

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12978513#action_12978513 ] 

Adam Kocoloski commented on COUCHDB-1004:
-----------------------------------------

The proposed solution, but I think you're overcomplicating matters with the code snippet

couch_util:get_value(mykey, List, couch_util:get_value(<<"mykey">>, List)) 

In that example the 'mykey' atom is already in the module, and the module is already loaded, so you know that couch_util:to_existing_atom/1 will give you an atom.  There's no need to check for the binary version.

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>         Attachments: COUCHDB-1004.patch
>
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Adam Kocoloski (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976565#action_12976565 ] 

Adam Kocoloski commented on COUCHDB-1004:
-----------------------------------------

Yep, and that's exactly how couch_util:to_existing_atom/1 works (sorry, my initial comment just said to_existing_atom which was stupidly vague).

> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (COUCHDB-1004) list_to_existing_atom is too restrictive as used by couch_rep

Posted by "Bob Dionne (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/COUCHDB-1004?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12976548#action_12976548 ] 

Bob Dionne commented on COUCHDB-1004:
-------------------------------------

Actually I'm not sure we'd want this, if in fact there's no problem with list_to_existing_atom (see email from Klaus about COUCHDB-829)

I looked at the existing couch_util:to_existing_atom  as well as all the places it's used. The existing function returns the string if it doesn't match an existing atom. That seems ok in most of the uses, but .eg. in couch_rep_writer it's not clear that it works correctly

~/emacs/couchdb:master$ grep couch_util:to_existing_atom src/*/*.erl
src/couchdb/couch_httpd.erl:        Meth -> couch_util:to_existing_atom(Meth)
src/couchdb/couch_httpd.erl:        'POST' -> couch_util:to_existing_atom(MethodOverride);
src/couchdb/couch_httpd.erl:        couch_util:to_existing_atom(MochiReq:get(method)),
src/couchdb/couch_os_process.erl:        throw({error, {couch_util:to_existing_atom(Id),Reason}});
src/couchdb/couch_os_process.erl:        throw({couch_util:to_existing_atom(Id),Reason});
src/couchdb/couch_rep_writer.erl:        ErrId = couch_util:to_existing_atom(Error),
src/couchdb/couch_rep_writer.erl:    ErrId = couch_util:to_existing_atom(couch_util:get_value(<<"error">>, Props)),
src/couchdb/couch_uuids.erl:    case couch_util:to_existing_atom(AlgoStr) of


> list_to_existing_atom is too restrictive as used by couch_rep
> -------------------------------------------------------------
>
>                 Key: COUCHDB-1004
>                 URL: https://issues.apache.org/jira/browse/COUCHDB-1004
>             Project: CouchDB
>          Issue Type: Bug
>          Components: Replication
>         Environment: erlang
>            Reporter: Bob Dionne
>            Priority: Minor
>
> We'd like to additional information to db_info in BigCouch, such as the Q and N constants for a given database. This causes replication to fail when replicating from BigCouch to CouchDB due to the use of list_to_existing_atom in couch_rep:dbinfo(...
> The claim is that list_to_atom pollutes the atoms table, however superficial testing indicates this is not the case, list_to_atom when called repeatedly seems to work fine. If this is true then consider reverting list_to_existing_atom back to list_to_atom.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.