Hi Everyone
I am wondering why default visibility constructors were added to each of the
*Type classes in the db.marshal package? If I'm understanding right, this
breaks the CLI.
Explanation:
This is the original UTF8Type class from 0.6.2, which worked:
import java.io.UnsupportedEncodingException;
public class UTF8Type extends AbstractType
{
public int compare(byte[] o1, byte[] o2)
{
try
{
return new String(o1, "UTF-8").compareTo(new String(o2,
"UTF-8"));
}
catch (UnsupportedEncodingException e)
{
throw new RuntimeException(e);
}
}
public String getString(byte[] bytes)
{
try
{
return new String(bytes, "UTF-8");
}
catch (UnsupportedEncodingException e)
{
throw new RuntimeException(e);
}
}
}
This is the UTF8Type class from the trunk and at least back to
2010-06-05_12-31-16, which does not work:
import java.io.UnsupportedEncodingException;
public class UTF8Type extends AbstractType
{
public static final UTF8Type instance = new UTF8Type();
UTF8Type() {} // singleton
public int compare(byte[] o1, byte[] o2)
{
try
{
return new String(o1, "UTF-8").compareTo(new String(o2,
"UTF-8"));
}
catch (UnsupportedEncodingException e)
{
throw new RuntimeException(e);
}
}
public String getString(byte[] bytes)
{
try
{
return new String(bytes, "UTF-8");
}
catch (UnsupportedEncodingException e)
{
throw new RuntimeException(e);
}
}
}
The difference is that after 0.6.2, these two lines were added:
public static final UTF8Type instance = new UTF8Type();
UTF8Type() {} // singleton
This causes the CLI to fail on gets on both Linux and Windows:
[defa...@keyspace1] set Standard2['mykey']['mycol']='My value'
Value inserted.
[defa...@keyspace1] get Standard2['mykey']['mycol']
Exception Class org.apache.cassandra.cli.CliClient can not access a member
of class org.apache.cassandra.db.marshal.UTF8
Type with modifiers ""
[defa...@keyspace1]
As you know, the Exception means that some classloader can't work with the
constructor with default (reduced) visibility.
So some code is definitely relying on the instance field, but it seems that
some other client code also relies on the (implicit) public constructor?
This is the same for all types that have similar "singleton" code added; I
use UTF8Type as representative example.
So I wonder what the reason was for preventing the implicit public
constructor by reducing visibility this way. Ostensibly, it's to provide a
Singleton,
but because the constructor is not marked private, this is not a true
singleton anyway, and because there are no instance fields in any of these
classes, everything's method local, there's nothing to be a Singleton about.
So what's the presumed Singleton part, even if it didn't break stuff?
I'm not sure what the benefit is, and this appears to be a bug. Perhaps I
misunderstand.
The following illustrates that removing this constructor, whose value is not
immediately clear to begin with, fixes the problem:
Type 'help' or '?' for help. Type 'quit' or 'exit' to quit.
[defa...@unknown] use Keyspace1
Authenticated to keyspace: Keyspace1
[defa...@keyspace1] set Standard2['k']['c']='v'
Value inserted.
[defa...@keyspace1] get Standard2['k']['c']
******************* ---->>> GETTING STRING WITH MODIFIED CODE.
=> (column=c, value=v, timestamp=1276287830318000)
[defa...@keyspace1]
But that may not be desired, and perhaps the real solution is to update
something in the CLI code that's using reflection.
Should I file a bug and, unless I don't understand the benefit of this,
supply the (very modest) fix for each of the types? Or is this known and
understood, simply broken for now, and going somewhere good? If so, where?
Thanks for such a great project and community.
Eben
--
"In science there are no 'depths'; there is surface everywhere."
--Rudolph Carnap
